search

pinax / pinax-stripe-light

a payments Django app for Stripe
MIT License
686 stars 285 forks source link

What does the new Stripe Billing mean for Pinax Stripe? #558

Open iMerica opened 6 years ago

iMerica commented 6 years ago

Hi,

Can you let me know what the plan is for Pinax Stripe? It looks like some of Stripe's entities and relationships have changed as well as the API itself.

See https://stripe.com/docs/billing/migrating.

The largest functional difference in version 2018-02-05 is the ability to represent multiple prices for the same product. Multiple Plans for a Product are only available from API version 2018-02-05. Until you upgrade your API Version to 2018-02-05 or later, you won’t be able to add multiple plans to the same product from the dashboard.

Thank you!

paltman commented 6 years ago

@iMerica That's a great question. Was just seeing Billing this weekend but haven't had time to digest it all.

hgezim commented 6 years ago

@paltman Any thoughts on this?

paltman commented 6 years ago

@hgezim still haven't had time to investigate

npetrides commented 6 years ago

@paltman Any plans to investigate?

mmic commented 6 years ago

@paltman @blueyed pinax-stripe's API is back at 2015. Since then, Stripe has performed 20+ new releases of the API, 3 of which were major and broke the API. One of them, released 6 months ago, was a substantial break as it split a model.

I am available to provide a PR, provided you have not already started to work on that and the PR is welcome. This will address issues #558 #571 #572 , and perhaps more.

I'd like to double-check this with you explicitly because the change is extensive: API version 2018-02-05 in particular splits Plans into Products and Plans, so this change will go all the way to models.

blueyed commented 6 years ago

@mmic From my point of view it would be awesome if you could provide a PR, and I would assume that @paltman has not started working on it already.

iMerica commented 6 years ago

I spent a few hours studying Stripe Billing and speaking with their sales team. The short version is that I think this package is mostly obsolete other than providing some helpful webhooks endpoints.

In the Stripe Billing universe, all of these data models live in Stripe. The only thing you need in your local Django app is the Stripe Customer ID which you can use to get related data like cards, invoices, subscriptions etc using the official Stripe Python client.

To me the choice is clear:

Use the official Stripe Python client to interact with latest/greatest data on Stripe's servers. Not worry about API version upgrades or synchronizing state.

vs

Use the models in Pinax Stripe to interact with potentially stale data in your local database and worry about API version changes, dunning and other things that are handled on your behalf with Stripe Billing.

blueyed commented 6 years ago

Thanks for your insights, @iMerica.

However, I think pinax-stripe could then be considered obsolete / not necessary before already, since everything lived in Stripe already, and could be accessed using stripe-python already. But the main benefit of it is its caching and the intermediate layer that does not require you to go through the Stripe API itself.

But I agree that it is quite a hassle in general, and that the project is lacking maintenance manpower.

We've started using it, and did a lot of work for the Connect part, but still have a bunch of changes that we have to track in our own branch/fork.

The project certainly would need some serious cleanup, e.g. with regard to bitcoin (not supported anymore), cards etc.

There is also https://github.com/pinax/pinax-stripe/pull/525 in an attempt to update the API already, but it got no feedback from the maintainer(s) since November last year.

npetrides commented 6 years ago

@mmic I agree with @blueyed that a PR would be awesome. Have you heard from @paltman?

mmic commented 6 years ago

I basically have to agree with iMerica. This app, like all django app, attempts to bring the Stripe world into django. Stripe has a knack for updating their API multiple times a year, including breaking changes. Making the two worlds coexist requires quite substantial maintenance manpower, which is mostly a waste of energy, and would unlikely be welcome by website developers (think 5 database schema changes per year).

Sticking with older versions of the API, at the same time, is not really an option, since it's a pain for new users, who create an account set by default with new API version, and which cannot be reverted.

I finally made a small app which makes bare-minimal assumptions on the stripe's API:

Looking back at the history of changes at Stripe's API, this approach would have survived the past multiple years, except for the tectonic data model break from 2017-04-06 .

blueyed commented 6 years ago

@mmic Sounds good. Any plans to publish it?

blueyed commented 6 years ago

Just for reference, there are plans for an upcoming sprint / discussion about backward compatiblity at https://github.com/pinax/pinax-stripe/issues/594.

paltman commented 6 years ago

I have been thinking a lot about this recently. The maintenance cost of having all the data modeled for caching purposes is very high.

It makes me wonder if we shouldn't consider getting rid of basically all models except for the Event for capturing/processing webhooks and leave it up to the site builder to construct and maintain whatever caching they need. This is a MASSIVE departure in years of thinking and implementation here but it is a painful maintenance issue and we don't even support the full menu of objects/functions yet.

I do believe caching is important for good web application performance, but it seems like it might be far better to be a site builder decision than something done generically in the app.

blueyed commented 6 years ago

Sounds good to me. It would be helpful if base classes / models would be available though, of course. What do you think about all the actions then? Should they get removed, and Stripe's API then used directly?

paltman commented 6 years ago

That's an interesting proposition and would significantly lower the amount of code we needed to maintain. A big reason a lot of the actions were written was to wrap the stripe Python client calls so we could bundle them with calls to sync/update local models. There is probably a lot of unnecessary mapping being done because of this.

I don't want to rush into this decision because it's going to be a massive amount of work and have significant impact for how this app is integrated with (will actually increase the burden on site developer but I also think give more freedom).

paltman commented 6 years ago

And what we are talking about is much broader than the title of this issue now. This isn't just about Billing which is really just Stripe's moving the Subscriptions documentation under a new product name and adding Product and Plan modeling. There are things like Connect, Charges, Invoices and more that are all impacted through this "rethink".

paltman commented 6 years ago

Just browsing through the code and I think we can kill 90% or more of the actions, there might be a couple that are more "utility" type functions we can keep or move to utils.py. The base models/mixins seem worth shipping. The Event, EventProcessingException and was thinking the Customer and UserAccount models but the vestiges in there to support multiple use cases (Customer.user versus Customer.users) makes me think that's a smell and we should leave to site developer and maybe just have a. CustomerBase model that the Event can link to.

I also think we get rid of all the views except for the Webhook view since the views depend on the models. We could eventually add back a set of base class Views meant to be extended but the views/templates have been hard to maintain as well and seem to be very site specific.

I'll continue to think on this and maybe start a wiki page with a plan but would love other's input on this as well. I'm pretty excited about the new direction as I think it's going to make this leaner, easier to maintain, and give the site developer more power.