search

balanced / balanced-ruby

Balanced API library in ruby.
MIT License
111 stars 47 forks source link

adds the ability to send the meta attribute in several calls that it was... #169

Closed bcatherall closed 10 years ago

bcatherall commented 10 years ago

... left out on.

Pretty simple fix, there were several call (especially when using orders) that only allow specific options to be included. This simply adds the meta attribute to those allowed options.

Other calls simply pass the options directly to Balanced, so they were allowing the meta attribute to be assigned.

I didn't include any tests for this, since there are no other examples of tests for optional attributes in there, but all the current test were still passing with this change.

remear commented 10 years ago

@bcatherall Thanks for this. As we've been discussing in https://github.com/balanced/balanced-ruby/pull/170, the right thing to do here would be to pass the options through. Would you be able and willing to make those changes? Also, this can be tested the same way as mentioned in that pull request by passing an optional parameter and make sure it's set correctly on the returned resource.

bcatherall commented 10 years ago

Ok, I've redone my changes to be more inline with #170. I also added some additional testing to make sure "meta" can be included in some other calls.

mjallday commented 10 years ago

lgtm!

mahmoudimus commented 10 years ago

if tests pass, +1

bcatherall commented 10 years ago

Travis failed in an odd way. Only 1.9.2 failed, and it was in a area that shouldn't have had a problem.

I'll switch my dev to 1.9.2 and see If I can recreate it.

bcatherall commented 10 years ago

After wasting a bit of time trying to recreate this locally, I dug through the Travis logs. It looks like Travis had some sort of glitch on that last run.

There's build #437 that passed all specs. Then there's build #438 that fails on one test on Ruby 1.9.2. The only difference between these two builds was the removal on a puts statement at lib/balanced/resources/credit.rb on line 17.

I really don't think the removal of a puts statement could cause the 500 error that the test failure saw.

remear commented 10 years ago

@bcatherall Yeah, that error doesn't look related to anything you did.

remear commented 10 years ago

@bcatherall I restarted the travis build for 1.9.2 and it passed fine. Everything looks good. Thanks for this! Please send an email to support@balancedpayments.com with your address and t-shirt size so we can send you some loot for your work.

mjallday commented 10 years ago

:+1: love this pull-request. thanks @bcatherall