concretecms-community-store / community_store_stripe

Stripe payment add-on for Community Store for concrete5
MIT License
7 stars 2 forks source link

Ominpay is big and complex - could it be replaced with a lighter library? #2

Closed Mesuva closed 7 years ago

Mesuva commented 7 years ago

This works fine with Omnipay, but it's a big library - surely there are lighter Stripe libraries out there?

mnakalay commented 7 years ago

Hi Ryan,

In #130 I said that maybe Payum would be a better alternative to Omnipay.

To my total shame I have to admit that after having tried for several days, I was absolutely unable to make it work.

I tested with Stripe and nothing... An order is recorded but no money is paid. I have no idea why.

Anyway. Concerning a lighter Stripe library, there are several libraries you can find on GitHub but they are more like one-time things that are not necessarily updated. Stripe, on the other hand, updates often.

Having said so, Stripe API is really easy to follow and use so maybe there is no need for another library.

There are really only 3 steps to the process: 1- set the API key 2- After javascript took care of card data and returned a token, charge the client using that token 3- check for success or failure and act accordingly.

Adding another library just for that might not be necessary?

I remember when Vivid had his Stripe plugin in the PRB and could never be bothered to make it work, it was using Stripe library directly and I was able to make it work. It was still the same 3 steps.

Mesuva commented 7 years ago

Yes, I reckon this just need to use the main Stripe library.

I released the Stripe add-on for eCommerce for 5.6.x, and that's all it used - wasn't too hard!

mnakalay commented 7 years ago

Hey Ryan, I meant to branch and then send a pull request and instead, I modified it directly. Sorry for that. But it works...

Anyway, Omnipay is gone and it's using Stripe PHP library directly now. I tested with stripe.js (custom form) and with their own form and it's all good.

Also, I show a descriptive error message when it's a card problem but for everything else, I show a generic error (which you should probably make nicer) and log the details for admins to check.

Oh and you probably should update the language file as I'm not sure how to do that

Mesuva commented 7 years ago

Fantastic work, I'll review and push up a release file when I have the chance!

dbuonomo commented 7 years ago

Just a quick question on this update. My understanding is that Omnipay is a general purpose payment layer between Community Store and the payment processor that is desired. It abstracts the vendor specific payment APIs into a common payment API. What are the implications of no longer having Omnipay as part of Community Store? How are the various payment gateway APIs wired into Community Store?

I'm not questioning the decision to do this as I understand that Omnipay is not necessarily simple and falling behind. Good work mnakalay.

mnakalay commented 7 years ago

@dbuonomo Omnipay is not actually part of Community store. Out of 2 payment add-ons (Paypal and Stripe) only Stripe was using it and the Stripe add-on was providing Omnipay.

Having removed Omnipay from the Stripe add-ons doesn't mean others can't develop their own payment solutions using Omnipay. there's nothing in the code that would prevent it

mnakalay commented 7 years ago

@Mesuva sorry I don't help more. You're doing an incredible work for all of us.

Mesuva commented 7 years ago

@mnakalay ha, what a silly apology, any/all input to this project is valuable and you've just pushed up a big improvement to this add-on!

Mesuva commented 7 years ago

Just gave this a test, all seemed to work well. I've pushed up a release