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

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

Open
iMerica opened this issue Apr 6, 2018 · 17 comments
Open

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

iMerica opened this issue Apr 6, 2018 · 17 comments

Comments

@iMerica
Copy link

iMerica commented Apr 6, 2018

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
Copy link
Contributor

paltman commented Apr 9, 2018

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

@iMerica iMerica changed the title What does the new Stripe Billing Means for Pinax Stripe? What does the new Stripe Billing mean for Pinax Stripe? Apr 11, 2018
@hgezim
Copy link

hgezim commented May 26, 2018

@paltman Any thoughts on this?

@paltman
Copy link
Contributor

paltman commented Jun 1, 2018

@hgezim still haven't had time to investigate

@npetrides
Copy link

@paltman Any plans to investigate?

@mmic
Copy link

mmic commented Jul 12, 2018

@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
Copy link
Contributor

blueyed commented Jul 12, 2018

@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
Copy link
Author

iMerica commented Jul 13, 2018

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
Copy link
Contributor

blueyed commented Jul 13, 2018

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 #525 in an attempt to update the API already, but it got no feedback from the maintainer(s) since November last year.

@npetrides
Copy link

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

@mmic
Copy link

mmic commented Jul 30, 2018

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:

  • the DB only hosts the User -> Stripe User ID association (that's always been stable so far), and the Event IDs (arrival timestamp + StripeEventID only), the latter mostly for troubleshooting purposes, being able to look up the event causing issues at Stripe for full details.
  • extraction of fields from webhooks assumes no specific location, it simply traverses the whole object tree and filters by name and value pattern.

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
Copy link
Contributor

blueyed commented Jul 30, 2018

@mmic
Sounds good.
Any plans to publish it?

@blueyed
Copy link
Contributor

blueyed commented Oct 9, 2018

Just for reference, there are plans for an upcoming sprint / discussion about backward compatiblity at #594.

@blueyed blueyed mentioned this issue Oct 9, 2018
@paltman
Copy link
Contributor

paltman commented Oct 10, 2018

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
Copy link
Contributor

blueyed commented Oct 10, 2018

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
Copy link
Contributor

paltman commented Oct 10, 2018

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
Copy link
Contributor

paltman commented Oct 10, 2018

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
Copy link
Contributor

paltman commented Oct 10, 2018

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.

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

No branches or pull requests

6 participants