Topher/more sales tax updates #9
Conversation
| # Go ahead and create the subscription | ||
| Stripe::Subscription.create(sub_opts) | ||
|
|
||
| # Now save the customer's plan | ||
| customer.plan = plan.stripe_id | ||
| customer.save |
There was a problem hiding this comment.
Were these two lines responsible for creating the subscription w/in Stripe? I tried to find any mention of this in the Stripe API docs and did not see anything about how a subscription gets created by setting a plan id on the customer.
Can these be removed now? does making a stripe subscription and passing in the customer and item[{plan:}] like you did properly associate the stripe sub and customer objects?
There was a problem hiding this comment.
So based on my memory and some digging, yes, the old version of the API allowed us to send a plan_id with the customer on creation and it would create the subscription to that plan.
Based on some of the Changelog docs I'm reading, that appears to be the case (https://stripe.com/docs/upgrades#2015-10-16):
Returns an error if a
tax_percentis provided without aplanduring a customer update or creation.
I take this to mean that at one point, you could pass along both the plan and tax percent when creating a customer and Stripe would take that and create the necessary objects. I think we can test it if need be.
To answer your question about removal: removing that made some specs fail; I tried that first (I believe the specs were expecting the plan to have been set).
I'm happy to dig into this some more; I definitely don't want to push out code that will cause issues! My hope was to get this on staging and do some testing to make sure we don't get duplicate charges or something weird like that. I also need to test the order bumps; I can't seem to get those to work locally.
There was a problem hiding this comment.
This sounds good.
I would still like to see PR #8 get used in mothership for a day or so before the new commit ref for this PR gets used too.
But thats a mothership issue and not a /koudoku issue.
Lgtm
Add some additional code necessary for the sales tax updates. Basically another place to call the
subscription_optionsmethod.