refactor: team and org billing portal service#24497
Conversation
WalkthroughThis pull request refactors subscription validation logic by centralizing two validation methods into the base Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionId.ts (1)
1-12: Add JSDoc documentation for the exported helper.Consider adding JSDoc to describe the function's purpose, parameters, and return value for better maintainability.
/** * Validates and extracts the subscription ID from team metadata. * * @param metadata - The team metadata as Prisma.JsonValue * @returns The subscription ID if valid, null otherwise */ export const getValidatedTeamSubscriptionId = (metadata: Prisma.JsonValue) => { // ... };packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionIdForPlatform.ts (1)
1-7: Consider simplifying or inlining this trivial helper.This function is a simple null-check that could be expressed as
subscriptionId || nullorsubscriptionId ?? null. Consider whether extracting such a trivial operation into a separate module adds meaningful value or if it would be clearer to inline at call sites.If you choose to keep the helper, consider adding JSDoc:
/** * Validates and returns the platform subscription ID. * * @param subscriptionId - The subscription ID to validate * @returns The subscription ID if truthy, null otherwise */ export const getValidatedTeamSubscriptionIdForPlatform = (subscriptionId?: string | null) => { 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 (4)
packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionId.ts(1 hunks)packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionIdForPlatform.ts(1 hunks)packages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.ts(2 hunks)packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts(2 hunks)
🧰 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/organization/OrganizationBillingPortalService.tspackages/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/organization/OrganizationBillingPortalService.tspackages/app-store/stripepayment/lib/getValidatedTeamSubscriptionIdForPlatform.tspackages/app-store/stripepayment/lib/getValidatedTeamSubscriptionId.tspackages/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/organization/OrganizationBillingPortalService.tspackages/app-store/stripepayment/lib/getValidatedTeamSubscriptionIdForPlatform.tspackages/app-store/stripepayment/lib/getValidatedTeamSubscriptionId.tspackages/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/organization/OrganizationBillingPortalService.tspackages/app-store/stripepayment/lib/getValidatedTeamSubscriptionIdForPlatform.tspackages/app-store/stripepayment/lib/getValidatedTeamSubscriptionId.tspackages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts
🧬 Code graph analysis (2)
packages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.ts (2)
packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionIdForPlatform.ts (1)
getValidatedTeamSubscriptionIdForPlatform(1-7)packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionId.ts (1)
getValidatedTeamSubscriptionId(4-12)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts (2)
packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionIdForPlatform.ts (1)
getValidatedTeamSubscriptionIdForPlatform(1-7)packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionId.ts (1)
getValidatedTeamSubscriptionId(4-12)
🔇 Additional comments (4)
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts (2)
4-5: LGTM! Clean extraction of validation logic.The import of shared helper functions successfully consolidates the validation logic and removes code duplication.
38-38: Consider removingawaitif helper becomes synchronous.If the suggestion to remove
asyncfromgetValidatedTeamSubscriptionIdis accepted, thisawaitcan also be removed for consistency.packages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.ts (2)
4-5: LGTM! Consistent refactoring across services.The shared helper functions are now being used consistently in both organization and team billing services, successfully eliminating code duplication.
43-43: Consider removingawaitif helper becomes synchronous.If the suggestion to remove
asyncfromgetValidatedTeamSubscriptionIdis accepted, thisawaitcan also be removed for consistency.
packages/app-store/stripepayment/lib/getValidatedTeamSubscriptionId.ts
Outdated
Show resolved
Hide resolved
packages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.ts
Outdated
Show resolved
Hide resolved
packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts
Outdated
Show resolved
Hide resolved
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
sean-brydon
left a comment
There was a problem hiding this comment.
Lets put these functions on the base service instead of creating a new util for them
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/app-store/stripepayment/lib/services/base/BillingPortalService.ts (1)
77-82: Consider simplifying the null check.The method can be reduced to a single line using the nullish coalescing operator.
Apply this diff to simplify:
protected 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 (3)
packages/app-store/stripepayment/lib/services/base/BillingPortalService.ts(2 hunks)packages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.ts(1 hunks)packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app-store/stripepayment/lib/services/team/TeamBillingPortalService.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/base/BillingPortalService.tspackages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.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/base/BillingPortalService.tspackages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.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/base/BillingPortalService.tspackages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.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/base/BillingPortalService.tspackages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.ts
⏰ 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). (3)
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
🔇 Additional comments (4)
packages/app-store/stripepayment/lib/services/base/BillingPortalService.ts (2)
9-10: LGTM! Imports support the new validation methods.The new imports are necessary for the validation methods added below.
67-75: LGTM! Solid validation logic.The method correctly parses team metadata using Zod schema validation and safely extracts the subscription ID. The early return pattern makes the logic clear.
packages/app-store/stripepayment/lib/services/organization/OrganizationBillingPortalService.ts (2)
34-36: LGTM! Correctly calling synchronous validation.The method is called synchronously without
await, which is correct sincegetValidatedTeamSubscriptionIdForPlatformdoesn't return a Promise. Good fix from the previous review.
41-41: LGTM! Proper use of base class validation.The refactor successfully delegates validation to the base class method, eliminating code duplication.
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
E2E results are ready! |
What does this PR do?