Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Paddle Billing #838

Merged
merged 33 commits into from
Sep 25, 2023
Merged

Conversation

deanpcmad
Copy link
Contributor

@deanpcmad deanpcmad commented Aug 14, 2023

This PR adds support for Paddle Billing, the new service/APIs available from Paddle. More Info

  • Old Paddle code has been renamed to Paddle Classic
  • Existing subscriptions will need to have their processor changed from paddle to paddle_classic

Paddle Billing now supports multiple subscriptions for a customer, similar to Stripe, so I've copied some code from Stripe and modified as required.

This uses my Paddle Gem which supports both the new Billing but also the Classic APIs. The existing Paddle Classic code is untouched, but could be changed to use my library.

Fixes #837

@excid3
Copy link
Collaborator

excid3 commented Aug 15, 2023

Dang! I will try and find some time this week to review.

The existing Paddle Classic code is untouched, but could be changed to use my library.

Let's do this in a second PR (before release). Consolidating dependencies would be nice and I'm sure that users on Paddle Classic will likely want to upgrade so they would only need the single dependency in the new version.

@deanpcmad
Copy link
Contributor Author

It's not complete yet, just thought I'd create a PR of what I've done so far 😅

Let's do this in a second PR (before release). Consolidating dependencies would be nice and I'm sure that users on Paddle Classic will likely want to upgrade so they would only need the single dependency in the new version.

Sounds good 👍

@strzibny
Copy link

strzibny commented Aug 26, 2023

@deanpcmad It's great that you already started a PR.

I too looked into Paddle Billing recently. For one, I already documented the new webhook verification so you can reuse it for the PR: https://nts.strzibny.name/verify-paddle-billing-webhooks-rails/

I have one question. Should we design the pause & cancellations changes as they are in Paddle Billing?

In Paddle, subscriptions are active even on a grace period which looks like Pay now follows as well. Hovewer it seems to me that Pay still marks subscriptions as "paused" or "cancelled" even if the change is only scheduled.

What would be the right way for this PR to work?

I would maybe prefer to have this same as Paddle since it's less mental overhead. On the other hand I understand the value of doing it similarly as for other providers.

@excid3 can you provide any insight on the design here? (I might just misunderstand it.)

@deanpcmad
Copy link
Contributor Author

@strzibny thanks for the link!

I'm not sure how the paused and cancelled subscriptions would work tbh. I see that they created a "scheduled change" with the effective_at field, which I'm currently using.

@strzibny
Copy link

strzibny commented Sep 6, 2023

Yep, exactly as you said, they should be modeled with scheduled changes I believe. But previously Pay would immediately marked a scheduled subscription to be paused or cancelled. I would like to avoid that and match the status in Paddle, whatever it is.

@deanpcmad
Copy link
Contributor Author

Maybe setting the pause_starts_at field when a pause event happens and then when the webhook fires when the pause actually happens, then update the status to paused?

Also, when upgrading a subscription, Paddle creates a Credit, so when creating transactions that will need to be taken into account

wn5XzLOSQG

@deanpcmad
Copy link
Contributor Author

Not sure why the tests are failing at the mo

@excid3 excid3 self-assigned this Sep 25, 2023
@deanpcmad
Copy link
Contributor Author

I received an email about a comment on this but I can't see it now 🧐
But no, Classic and Billing use different ways of verifying the signature.
I've not had much chance to work on this, but I can give you access to my repo?

@excid3 excid3 merged commit 1d21093 into pay-rails:master Sep 25, 2023
0 of 33 checks passed
@excid3
Copy link
Collaborator

excid3 commented Sep 25, 2023

Whoopsie. I did not mean to merge that lol. 🙃

@excid3
Copy link
Collaborator

excid3 commented Sep 25, 2023

I did get tests passing again though. What other features are unfinished?

@deanpcmad
Copy link
Contributor Author

haha, I did wonder that. I'm pretty sure all the subscription stuff is working.

Need to work out how Pay will show paused/cancelled subscriptions as Paddle use "scheduled changes". e.g. If I set a subscription to cancel, the status will still be active but there will be a scheduled_change option showing the action and the effective_at.
On Paddle Classic, Pay was set to change the status straight away but @strzibny said it might be best to change this for Paddle?

The documentation also need updating.

You may notice that I've removed the passthrough code for Paddle too as it's like Stripe, so a Customer will be created on Paddle and then a subscription will be linked to that customer

@deanpcmad
Copy link
Contributor Author

Also need to work on tests for Paddle

@excid3
Copy link
Collaborator

excid3 commented Sep 25, 2023

I see that PaddlePay is still referenced in the new Paddle code, so we should update all those references as well.

All we need for the scheduled changes is to use those in the sync method to determine the date, just like we do with Stripe. Should be easy enough.

@excid3
Copy link
Collaborator

excid3 commented Sep 25, 2023

@deanpcmad I see the custom_data.passthrough is still referenced in Subscription#sync. That can be removed too?

@deanpcmad
Copy link
Contributor Author

Sounds good, so still set the status and date fields as required? I believe I already have that set when running pause or cancel on the subscription, so that can be added so the sync method too.

And yes, that can be removed as I found that it's much easier and cleaner to set the Customer ID when setting up the Paddle subscription button.

I can help work through the docs, as there's a few changes on their new Paddle.js

@excid3
Copy link
Collaborator

excid3 commented Sep 25, 2023

Yeah, status should always be set to what Paddle has defined.

I'll go ahead and remove that. 👍

Docs + tests are going to be the biggest things to complete I think.

@excid3
Copy link
Collaborator

excid3 commented Sep 25, 2023

I think that charge needs either an update as it requires a Price ID now and does not use amounts.

@excid3
Copy link
Collaborator

excid3 commented Oct 26, 2023

After upgrading Jumpstart Pro... I'm starting to think it's better if we use Pay::PaddleBilling and Pay::PaddleClassic. Much easier to differentiate and less confusing for existing users. They're likely to keep that naming in their site / docs for a long time.

I will probably rename it today or tomorrow unless you beat me to it @deanpcmad.

@deanpcmad
Copy link
Contributor Author

Yeah that makes sense tbh. Some tests need to be written for it too

@deanpcmad deanpcmad deleted the paddle-billing branch October 26, 2023 14:42
@excid3
Copy link
Collaborator

excid3 commented Oct 26, 2023

I'll start a PR for it today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Paddle Billing
3 participants