Paddle payment provider #165
Conversation
|
Wow, this is awesome @nm 🙏 I'm finishing up moving to a new place, so it may be a couple days before I can fully review this. One thing we definitely need is a better documentation system than the README, especially with the introduction of Paddle since there are a decent bit of unique differences / examples that will need to go along with it. |
|
I'm using this currently on building https://btfy.io. Seems alright! I haven't used in prod yet since I'm still working on the authorization. Looking forward for this to get merge :) |
|
Looks like we're gonna want to start putting together a more organized set of docs because Paddle introduces a lot of exceptions, and I'm sure we'll have plenty of exceptions as we get into marketplaces and other things. Maybe the wiki or github pages? I'll probably merge this shortly. I just wanted to find some time to try it out before I did. |
|
Yes, I think an separate page for each payment provider definitely makes sense. Paddle is different from Stripe or Braintree in many ways and therefore the documentation will also be different. At this stage I don't think github pages or wiki make a huge difference. The wiki would probably be sufficient for now, but choose whatever you prefer. I can then adapt the documentation for paddle with pleasure. In a further step, one would certainly have to investigate again to what extent the various payment providers can be combined and for which functions it makes sense. Regarding Paddle handling the webhooks was pretty much the same, but the customer and subscriptions objects were different. Please let me know if there is anything I can do to help. |
|
This looks great! Nice work @nm, I started working on a gem and was going to start working on the same integration before I noticed this PR |
| product: 12345, | ||
| passthrough: "{\"owner_id\": 98765, \"owner_type\": \"User\"}" | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
Alternatively, if using the paddle button:
<a href="#!" class="paddle_button" data-product="12345" data-email="<%= current_user.email %>" data-passthrough="<%= { owner_id: current_user.id, owner_type: "User" }.to_json %>"
There was a problem hiding this comment.
Is this secure? I could change the model and ID to anything.
Seems like we should do a signed GlobalID or something instead that we could verify server-side.
There was a problem hiding this comment.
Yes, you're right. That would allow to pay for another owner id or owner type. I changed it and now a signed GlobalID is used. In addition, a purpose string ensures that GlobalIDs cannot be swapped between Paddle products.
There was a problem hiding this comment.
Since this is received by the webhook for the subscription, I think the for: is probably unnecessary?
If so, we can remove it and simplify a little bit which is nice.
There was a problem hiding this comment.
It depends: If you can create subscriptions for multiple owner types in your app or signed global IDs are made public otherwise, the for: would be necessary to prevent swapping the SGID in the javascript code.
Paddle.Checkout.open({
product: 12345,
passthrough: "{\"owner_sgid\": \"<%= modelA.to_sgid.to_s %>\"}"
});
Paddle.Checkout.open({
product: 67890,
passthrough: "{\"owner_sgid\": \"<%= modelB.to_sgid.to_s %>\"}"
});How likely is such a scenario? Probably not very likely. To be honest: If I pay for a subscription I am not interesting in changing the owner. That is the reason I just used owner_type and owner_id in the beginning. I don't know, maybe using SGIDs without for: is a good tradeoff between simplicity and security. What do you think?
There was a problem hiding this comment.
At least with an SGID, they could only swap SGIDs they had access to since it'd have to be generated server side. That'd leave it only to accounts they have access to.
If it was clear text, it's easy to tamper with. To be fair, you wouldn't want to pay for something and get it assigned to someone else, so it seems illogical to tamper with.
Maybe I'll build a helper to generate the passthrough JSON.
|
I started integrating this locally and in my so far limited testing it seems to be working without issues. Not deployed to production yet though. |
|
Thanks @grk for the review! I've included your suggestions in the PR. |
|
Sorry for the delay on this, I got busy and I get married tomorrow so I'll dive into this once I get back from our honeymoon. Thanks for the work on this! It's looking good. |
|
+1 on this one. plus I'm thinking about using jumpstartpro which use itself pay gem. Would I be able then to use paddle via pay gem within jumpstart pro with minimum adjustement? |
|
|
||
| charge.update( | ||
| amount: Integer(data["sale_gross"].to_f * 100), | ||
| card_type: data["payment_method"], |
There was a problem hiding this comment.
payment_method will be one of card, paypal, free, apple-pay, wire-transfer, not sure if it fits in this field
There was a problem hiding this comment.
Yes, storing the payment method in a card type column is definitely misleading. I'll second that. But this is consistent to the existing Braintree integration which stores "PayPal" in the card_type column as well.
Maybe it would be better to have more general columns for payment method and payment method meta data. But I didn't want to alter too much with this PR.
There was a problem hiding this comment.
Yeah, we need to improve this in Pay at some point. Braintree stores "paypal" in card_type and the paypal email in card_last4 which isn't great.
| card_type: data["payment_method"], | ||
| receipt_url: data["receipt_url"], | ||
| created_at: DateTime.parse(data["event_time"]) | ||
| ) |
There was a problem hiding this comment.
Would be cool if we could store all the information from the webhook somehow. In postgres I'd create a jsonb column for that, but I'm not sure what would be an universal method for that.
There was a problem hiding this comment.
There was a problem hiding this comment.
JSON column would be nice, although compatibility is tough. I'm serializing and using text for sqlite, json for mysql, and jsonb for postgres in noticed and serializing with the ActiveJob coder. That works pretty well, but not queryable in all cases. I think that's probably okay. We can open up a separate issue for that.
|
Hello, thank you for the amazing work. I set it up on staging just to test it out and seems to be working, only issue is that when a user subscribes, the |
|
|
|
Since this PR adds columns unique to Paddle, this is probably a good opportunity to refactor and use json columns instead. I think I'm going to do that using what we've got in the Noticed gem. Other than that, I think this looks fantastic and will merge it right away. 👍 Thanks for your patience on this guys! 🙏 |
|
I ended up having to do a lot of changes for the json column. Will you guys check out this branch and let me know if it works? https://github.com/pay-rails/pay/tree/nm-paddle |
Why Paddle?
For many countries there are cases where the tax is due in the target country of a customer. This makes it obviously very complicated for small companies. Paddle acts on your behalf as what is called a Merchant of Record (MOR). That makes them different from Stripe or Paypal. If you don't want to deal with tax or invoicing issues, Paddle is an interesting alternative.
Integration
Paddle is different from Stripe and Braintree in several ways. This makes integration a bit challenging. I tried to adapt to the existing implementation structure as good as possible, but there are a few edge cases:
update_urlandcancel_urlto the subscription model. It is also possible to change certain fields via the API (plan change, quantity, cancellation, ..), but for updating payment details you definitely need the update URL.receipt_urlfield to the Charge Model. The field may also be useful for other external invoicing solutions.Paddle Gem
I also wrote a paddle gem which is integrated in the proposed integration.
The pull request further includes tests and an update of the
readme.md.I am happy to discuss the integration approach and can also do improvements or modifications if needed.