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

subscription_items on Subscription::update() is failing #554

Closed
jlomas-stripe opened this issue Jun 26, 2017 · 9 comments · Fixed by #596
Closed

subscription_items on Subscription::update() is failing #554

jlomas-stripe opened this issue Jun 26, 2017 · 9 comments · Fixed by #596

Comments

@jlomas-stripe
Copy link

Nested objects in an array are expected to all be the same shape, but that isn't the case for subscription_items, so the following won't currently work because it expects the second object to have an id attribute:

subscription_items: [{ :id => 'si_potato', :delete => true }, { :plan => 'radish' }]

Probably better to switch this up to just use indexed maps like PHP and Node do.

I'm hoping to take a run at this in the next couple of days, but anyone else please feel free to push me out of the way and jump on it. :)

@brandur-stripe
Copy link
Contributor

Arg, good catch. I'm a little surprised we didn't add support for this back when we decided to make the indexed map style calls kosher for subscriptions.

We should probably add an alternate encoding method with the expectation that will eventually be general purpose enough to be usable for encoding all parameters. I think the expectation right now is that although the indexed map style calls only work for subscription items (and maybe a few other places that have been moved to the new abstract API method infrastructure), eventually we'll support it on every endpoint.

@mattfordham
Copy link

mattfordham commented Sep 5, 2017

I believe I am experiencing a related issue. Given this:

subscription.items = [{plan: 'test-1', id: 'si_blah_blah'}, {plan: 'test-2'}]
subscription.save

I get the following error:

ArgumentError: All maps nested in an array should start with the same keys (expected starting key 'id', got 'plan')

So, it seems maybe the encoding is reordering the keys?
Is the above error expected?
Are there any recommended workarounds in the meantime?

@mattfordham
Copy link

Perhaps using SubscriptionItem (update|delete|create) is the "workaround"... but that'll obviously require a handful of separate API calls to accomplish the same thing.

@ob-stripe
Copy link
Contributor

ob-stripe commented Sep 11, 2017

Okay, so here's what's happening:

  • when setting the items attribute to be a list of hashes, the hashes are automatically converted into StripeObjects (here)
  • when initializing a StripeObject, the id attribute is special-cased in the constructor to be inserted first into the internal values hash (here)
  • when the StripeObject is converted back into a hash by serialize_params, the id attribute is thus returned first
  • when URL-encoding the parameters, the fact that the id attribute is returned first causes the issue described by @mattfordham in the array-of-hashes handler (here)

To illustrate:

subscription = Stripe::Subscription.new
subscription.items = [{plan: 'test-1', id: 'si_blah_blah'}, {plan: 'test-2'}]
subscription.serialize_params

returns:

{:items=>[{:id=>"si_blah_blah", :plan=>"test-1"}, {:plan=>"test-2"}]}

As a workaround, you can replace the list of hashes by a hash of hashes yourself. E.g. this should work:

subscription.items = {'0': {plan: 'test-1', id: 'si_blah_blah'}, '1': {plan: 'test-2'}}
subscription.save

(Edit: nevermind, the workaround doesn't actually work :( )

This should be handled internally by the client library itself though. We'll work on a fix.

@mattfordham
Copy link

Thanks @ob-stripe, very helpful. Also, good to hear about the "workaround". I'll try that!

@joebartels
Copy link

The workaround doesn't do it for me:

subscription.items = {'0': {deleted: true, id: 'si_blah_blah'}, '1': {plan: 'test-2'}}
subscription.serialize_params

{:items=>
  {:object=>"",
   :data=>"",
   :has_more=>"",
   :total_count=>"",
   :url=>"",
   :"0"=>{:deleted=>true, :id=>"si_blah_blah"},
   :"1"=>{:plan=>"test-2"}},
 :metadata=>{}}

and subscription.save gives:

Stripe::InvalidRequestError: (Status 400) (Request req_f5Bv7SMfohtOey) Invalid array

Am I missing something?
Using v3.3.2

@joebartels
Copy link

joebartels commented Sep 28, 2017

my work around has been to simply use the https API for this type of subscription update. Something like this:

items = [ { id: 'plan_1', deleted: true }, { plan: 'plan_2' } ]
query = {}

# this format worked well for the http request e.g.,
# {'0': { id: 'plan_1', deleted: true }, '1': { plan: 'plan_2' } }
items.each_with_index { |item, index| query[index] = item }

HTTParty.post(
  "https://api.stripe.com/v1/subscriptions/#{subscription_id}",
  query: query,
  headers: { 'Authorization': "Bearer #{api_key}" }
)

@ob-stripe
Copy link
Contributor

Hey everyone, I just wanted to let you know that we haven't forgotten about this. I can confirm that the workaround I offered above does not actually work because of a different issue in the library (sorry about that! I thought I tested it but apparently got confused).

For now, you can certainly bypass the gem and send the HTTP request to the API using another client as @joebartels suggested just above. This obviously isn't ideal and we'll work on a fix to ensure that the stripe gem does what's expected.

@brandur-stripe
Copy link
Contributor

Fixed released as part of 3.5.3.

snappy316 added a commit to choyt800/tpc_app that referenced this issue Apr 20, 2018
Update the Stripe Gem up a few versions (not to the latest one), so we
can fix [this issue](stripe/stripe-ruby#554).
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

Successfully merging a pull request may close this issue.

5 participants