search

laravel / cashier-stripe

Laravel Cashier provides an expressive, fluent interface to Stripe's subscription billing services.
https://laravel.com/docs/billing
MIT License
2.37k stars 671 forks source link

What versions of Stripe API is this tested against? #592

Closed Humni closed 5 years ago

Humni commented 5 years ago

Can the docs please be updated to show what versions of the Stripe API this package is tested against? Stripe updates APIs relatively often, and over the past year there have been some structural changes with the plans and subscriptions (multi-plan subscriptions, tho not yet supported by cashier #397 ) which affects the API.

Not that I need it, but this should also probably be noted down for brain tree too.

driesvints commented 5 years ago

Cashier doesn't fully implement everything the Stripe API does. If you want a feature to be implemented you can send in a PR.

Humni commented 5 years ago

@driesvints I understand that it specifically doesn't implement all the features, I'm not asking it to either. I'm asking to know what versions of the stripe API are known to work with laravel cashier, as this is required for enterprise maintenance planning.

The community sure can test it themselves, but if the package maintainers noted down what versions they tested against it'd save the community a lot of time and hassle. Happy to hear counter opinions to this, but I don't see any reasonable grounds for withholding this information

driesvints commented 5 years ago

At the moment we're using v6 of their SDK: https://github.com/laravel/cashier/blob/8.0/composer.json#L22

hackel commented 5 years ago

Unfortunately the SDK can still be used against multiple API versions. I think what is needed is an official, designated API target. E.g. "Cashier 9.0 is tested against API version 2017-04-06 and tested up to 2018-05-21."

I want to upgrade my site to API version 2018-11-08, and that would be much easier if there was an upper-limit of API versions guaranteed to be compatible with Cashier, then I can just read the Changelog and see if there are any breaking changes since then that might impact my application.

https://stripe.com/docs/upgrades#api-changelog

driesvints commented 5 years ago

The Stripe PHP SDK doesn't specifies a default version. Since our test suite doesn't either the latest API version is always used. See https://stripe.com/docs/api/versioning

https://github.com/stripe/stripe-php/blob/ba85ad7c1618462083a7e9a542ded929232e8bff/lib/ApiRequestor.php#L356-L358

The Test suite could use some more tests but other than that we always test against the latest API version afaik.

hackel commented 5 years ago

@driesvints I don't believe that's true. Because Stripe::apiVersion defaults to null, it will use whatever API version the Stripe account is set to unless I explicitly set Stripe::apiVersion myself. I don't believe the PHP SDK does any parameter validation either, it relies on the consumer to specify the correct parameters for whichever version they are targetting.

For example, in the latest API version, a Subscription::create request does not accept a "plan" parameter, it must be in "items". Yet Cashier's SubscriptionBuilder still uses "plan". Is it backward compatible? Probably, but I have no idea. It requires me to be aware of this particular call's internals.

I just want to be sure that if I upgrade my account's assigned API version, Cashier won't break. Hopefully that makes sense. I do find Stripe's approach to versioning rather frustrating in this regard.

driesvints commented 5 years ago

@hackel you are absolutely right. I'll check into this soon.

driesvints commented 5 years ago

@hackel would it at all be wanted that we'd simply lock the version ourselves by setting it in the serviceprovider by using Stripe::setApiVersion?

driesvints commented 5 years ago

@hackel I sent in a PR here. Can you check if that looks ok to you? https://github.com/laravel/cashier/pull/603

driesvints commented 5 years ago

We've updated the PR to only set the version in the test suite. This way people may still use a lower version if they'd like.

Humni commented 5 years ago

@driesvints #603 looks good to me, this is exactly what I was looking for.

driesvints commented 5 years ago

PR got merged so closing this. Thanks for bringing this up! :)

hackel commented 5 years ago

Thanks @driesvints, I didn't realize the tests were running against a live Stripe dev server as opposed to stubbing the api. That makes it easy to plug in whichever version you're using and just run the tests yourself.

driesvints commented 5 years ago

@hackel yeah exactly. Docs for setting everything up are in the readme btw.