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

Require Faraday 0.10 for proper nested array encoding #588

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

ob-stripe
Copy link
Contributor

r? @brandur-stripe
cc @stripe/api-libraries

Fixes #576.

Seems difficult to add a test for this as the issue doesn't happen in stripe-ruby itself. Maybe by setting up some sort of echo server? Not sure if worth the trouble, wdyt?

@brandur-stripe
Copy link
Contributor

Nice again OB! I found it a little hard to believe that a problem in decode could be the cause (why would you need to decode parameters during a request?), but I put a breakpoint in there, and yep, it sure is.

This helped me find another problem too. I have a temporary patch that tries to add integer-indexed hash arrays for subscription_items like this:

diff --git a/lib/stripe/invoice.rb b/lib/stripe/invoice.rb
index c865ec2..48d8ef1 100644
--- a/lib/stripe/invoice.rb
+++ b/lib/stripe/invoice.rb
@@ -7,6 +7,10 @@ module Stripe
     OBJECT_NAME = "invoice".freeze
 
     def self.upcoming(params, opts = {})
+      if params[:subscription_items]
+        params[:subscription_items] = Util.array_to_hash(params[:subscription_items])
+      end
+
       resp, opts = request(:get, upcoming_url, params, opts)
       Util.convert_to_stripe_object(resp.data, opts)
     end

Turns out, that code doesn't work! It's nothing to do with any bugs on our end, but that same code in Faraday sees an integer key (like 0) in outgoing parameters and it just throws it out! This is made worse because it doesn't show up in STRIPE_LOG or anything because as far as it knew, we were sending the right thing to our underlying HTTP library! I only caught the problem with Webmock.

Anyway, I'll fix that one separately.

@brandur-stripe brandur-stripe merged commit e4725b8 into master Oct 5, 2017
@brandur-stripe brandur-stripe deleted the ob-fix-576 branch October 5, 2017 16:47
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 this pull request may close these issues.

2 participants