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 Git ↗︎ 3 Skipped Deployments
|
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
|
|
||
| const handler = async (data: SWHMap["payment_intent.succeeded"]["data"]) => { | ||
| const paymentIntent = data.object; | ||
| // TODO: Implement payment intent succeeded webhook handler |
There was a problem hiding this comment.
I think I left it as a placeholder. RN we handle upgrades via redirect callback. I'm not sure if here we can upgrade expired subscriptions or similar.
…n.deleted.team-plan.ts Co-authored-by: Omar López <zomars@me.com>
|
@anikdhabal could we please get your help testing this one? |
Sure, I will test this 🙏 |
keithwillcode
left a comment
There was a problem hiding this comment.
Nice changes overall. Just have some suggestions on responsibilities and naming.
| teamId: z.coerce.number(), | ||
| }); | ||
|
|
||
| const handler = async (data: SWHMap["customer.subscription.deleted"]["data"]) => { |
There was a problem hiding this comment.
Well doesn't this code look clean, simple and to the point. 🙏
| parentId: true, | ||
| }); | ||
|
|
||
| export class TeamBilling { |
There was a problem hiding this comment.
I agree with @joeauyeung here. This feels more like a repository rather than a team domain object. We should rename imho.
In DDD, for example, a repository would be used to return data for a team. Then, using just a constructor, static methods like we have here or a factory, you take that data and construct a team object with it. Mixing the two responsibilities here is misleading to the caller.
| }); | ||
|
|
||
| describe("cancel", () => { | ||
| const internalTeamBilling = new InternalTeamBilling(mockTeam); |
There was a problem hiding this comment.
What does "Internal" mean here? Is it to describe that this class is handling most of the core logic for team billing? Looking at the class, seems like TeamBillingService would be more fitting since it's doing repository calls and logical changes. i.e. It's orchestrating like an Application Service.
There was a problem hiding this comment.
By internal. It means Cal.com internal billing.
| @@ -0,0 +1,27 @@ | |||
| import logger from "@calcom/lib/logger"; | |||
|
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ |
E2E results are ready! |
| }); | ||
| }); | ||
|
|
||
| describe("find", () => { |
keithwillcode
left a comment
There was a problem hiding this comment.
Changes since my feedback look better. We can still optimize not having the static repo on there but can be a follow-up.
Have discussed with zomars and we agreed to put onto qa for testing.
What does this PR do?
The goal of this PR is to centralize our internal billing in a shared service. This will allow us to have an overview of all the billing related logic in one place.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist