chargify / chargify_api_ares

A Chargify API wrapper for Ruby using ActiveResource
http://chargify.com
MIT License
161 stars 95 forks source link

@subscription.reactivate returning object instead of True/False #90

Closed jonathansimmons closed 10 years ago

jonathansimmons commented 10 years ago

Problem: A call to @susbcription.reactivate has stopped returning true/false but instead is returning the subscription itself.

Steps to reproduce: Try reactivating a cancelled subscription with the bogus card 2.

Expected response: False

Actual Response: The subscription object containing errors.

Misc thoughts

I'm seeing that the re-activate function here is wrapped in process_capturing_errors which I'm unsure of what this does. Maybe there was a change in how errors are handled in the gem I missed?

My expectation is that a @subscription.save or @subscription.reactivate call would return true/false like any standard ActiveRecord resource allow us to then inspect the errors on the object afterwards.

(cc: @yesthatallen)

jonathansimmons commented 10 years ago

Further review of process_capturing_errors it appears the resource_invalid errors are being intentionally rescued. Our only having recent updated to the latest chargify_api_ares version would mean we are just now being subjected to this change.

It still seems odd to me to just return the object rather than a true / false here. While it's not hard to simple look for @subscription.errors after the reactivate call it seems inconsistent with rails standards.

jonathansimmons commented 10 years ago

Closing this as It's obviously by design on Chargify's behalf. We've adjusted our billing actions as needed. @jeremywrowe I'd still love to hear about the reasons behind decision to return the object rather than true/false.

jeremywrowe commented 10 years ago

@jonathansimmons When designing the endpoint we designed it to match API standards not Rails standards. The gem simply wraps the API, there are other consumers of the API outside of just Ruby/Rails. Java, PHP, etc. We also wanted to provide reasons of why the reactivation failed so that issues can be resolved either internally by a merchant or externally by a customer. Also since reactivate doesn't fit into the normal REST standard we are doing something a bit abnormal yet trying to maintain usefulness.

YesThatAllen commented 10 years ago

Where's this all documented? (these details in particular)

jeremywrowe commented 10 years ago

@YesThatAllen https://docs.chargify.com/api-subscriptions under the reactivate section.

reactivate