refactor: implement DI in team billing service and team billing data repository factory#24803
Conversation
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
|
||
| import type { ITeamBillingRepository } from "./team-billing.repository.interface"; |
There was a problem hiding this comment.
File names used dashes, instead renamed to use camel casing. Inline with the rest of the codebase
There was a problem hiding this comment.
Used if team billing is not enabled
There was a problem hiding this comment.
Removed the test that touches the repository but instead test the factory
There was a problem hiding this comment.
New factory to return the prisma repository or the stub one if billing is not enabled
…ervice.test.ts Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
…e with TeamBillingService - Replace InternalTeamBilling with TeamBillingService - Use constructor injection with mock dependencies instead of factory pattern - Remove BillingRepositoryFactory mock and import - Update all test cases to use mockBillingProviderService, mockTeamBillingDataRepository, and mockBillingRepository - Simplify saveTeamBilling tests to focus on repository.create calls - All 11 tests now pass with the new DI structure Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
hariombalhara
left a comment
There was a problem hiding this comment.
LGTM !! Great refactor @joeauyeung !!
…ner's getBillingProviderService - OrganizationPaymentService now uses getBillingProviderService() from DI container - Test was mocking @calcom/features/ee/payments/server/stripe directly, which no longer works - Added mock for @calcom/features/ee/billing/di/containers/Billing module - Mock returns fake billing provider that delegates to mockSharedStripe - Preserves all existing test assertions and helpers - Fixed lint error by prefixing unused lastCreatedSessionId with underscore - All 11 tests now pass (1 skipped as expected) Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
What does this PR do?
This PR refactors the team/organization billing system to use dependency injection (DI) with the
ioctopuslibrary, following SOLID principles. The main changes include:ioctopusTeamBillingServiceandBillingProviderServicethat automatically returns stub implementations when billing is disabledKey architectural improvements:
getTeamBillingServiceFactory()- Factory for team billing operationsgetBillingProviderService()- Factory for Stripe billing servicegetTeamBillingDataRepository()- Repository for team billing data accessIS_TEAM_BILLING_ENABLED=falseMandatory Tasks (DO NOT REMOVE)
skipTeamTrials, but this is primarily a refactoring PRHow should this be tested?
Environment Setup:
IS_TEAM_BILLING_ENABLEDis set appropriatelySTRIPE_PRIVATE_KEYis configuredSTRIPE_ORG_PRODUCT_IDadded to turbo.jsonTest Scenarios:
Team Billing Operations (with billing enabled):
Billing Disabled Mode:
IS_TEAM_BILLING_ENABLED=falseWebhook Handling:
customer.subscription.deletedwebhookinvoice.paidwebhook for organizationsSubscription Status:
hasActiveTeamPlancorrectly identifies active/trialing/past_due subscriptionsskipTeamTrialsfor teams in trial periodExpected Behavior:
Review Checklist
Critical areas to review:
new TeamBilling(),new InternalTeamBilling(), andnew StripeBillingService()have been replaced with factory callsStripe.Subscription.Status(lowercase strings) toSubscriptionStatusenum (uppercase) is handled correctly in all comparisonsIS_TEAM_BILLING_ENABLED=falseand don't cause runtime errorsKnown Risks:
Link to Devin run: https://app.devin.ai/sessions/35e0b25ba04b4f59b50b4fef1a492411
Requested by: joe@cal.com (@joeauyeung)