chargify / chargify_api_ares

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

bank_account payment profile #80

Closed nickhammond closed 9 years ago

nickhammond commented 10 years ago

A subscription's payment profile can now be associated with a bank_account but calling payment_profile on a subscription will only return the credit_card payment profile.

https://github.com/chargify/chargify_api_ares/blob/master/lib/chargify_api_ares/resources/subscription.rb#L37

def payment_profile
  self.respond_to?('credit_card') ? credit_card : nil
end

A subscription can have both a bank_account and a credit_card as a payment profile but the more common case would be a subscription with one payment profile. Credit card is already the existing default so I suppose it would make sense to check for a bank account, if no bank account then look for a card and if nothing then nil. That would fit closest to the current implementation but wanted to get everyone's thoughts and see if another way is preferable.

jeremywrowe commented 10 years ago

@nickhammond Thanks for opening this up for discussion. payment_profile in this case is either the current (active) credit card or bank account on file. There can be multiple credit cards / bank accounts on file but we only return the active chargeable payment profile. bank_account and credit_card are (STI) classes that inherit from payment profile. We have not yet added bank to the gem yet but we should shortly.

nickhammond commented 10 years ago

I was going to add it and create a PR as I'm using the gem and we have ACH enabled now. Along with deleting 'bank_account' from the attributes as part of save on a subscription(https://github.com/chargify/chargify_api_ares/blob/master/lib/chargify_api_ares/resources/subscription.rb#L14), not able to get bank account information to update currently though.

As of right now for a subscription that has a bank account but no credit card payment_profile is nil.

irb(main):008:0> s.payment_profile
=> nil
irb(main):009:0> s.bank_account
=> #<Chargify::Subscription::BankAccount:0x007f93f30bfe58 @attributes={"bank_account_holder_type"=>"personal", "bank_account_type"=>"checking", "bank_name"=>"My bank", "billing_address"=>nil, "billing_address_2"=>nil, "billing_city"=>nil, "billing_country"=>nil, "billing_state"=>nil, "billing_zip"=>nil, "current_vault"=>nil, "customer_id"=>5299468, "customer_vault_token"=>nil, "first_name"=>"Diamond", "id"=>3424529, "last_name"=>"Dawes", "masked_bank_account_number"=>"XXXXXXX1", "masked_bank_routing_number"=>"XXXXXXX1", "vault_token"=>nil, "payment_type"=>"bank_account"}, @prefix_options={}, @persisted=false>
irb(main):010:0> s.credit_card
NoMethodError: undefined method `credit_card' for #<Chargify::Subscription:0x007f93f31df860>
        from /Users/nick/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/activeresource-3.2.13/lib/active_resource/base.rb:1470:in `method_missing'
        from (irb):10
        from /Users/nick/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/railties-3.2.13/lib/rails/commands/console.rb:47:in `start'
        from /Users/nick/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/railties-3.2.13/lib/rails/commands/console.rb:8:in `start'
        from /Users/nick/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/railties-3.2.13/lib/rails/commands.rb:41:in `<top (required)>'
        from script/rails:6:in `require'
        from script/rails:6:in `<main>'
jeremywrowe commented 10 years ago

Associations should be updated through their own endpoints as suggested in the documentation. When fetching data for a child association it should be considered read only. To update a bank accounts information please use the payment profile endpoint.

nickhammond commented 10 years ago

There's a whole section on the API docs for subscriptions talking about passing payment_profile_attributes(also known as credit_card_attributes or bank_account_attributes). Updating via subscription also just takes a single API call as opposed to updating the subscription and then updating the payment profile which is less work on us and your servers. I just tried updating the payment_profile directly via the API and it kicked back a 500.

p = Chargify::PaymentProfile.find(s.bank_account.id) 
p.update_attributes(:bank_name => "New Bank", :bank_account_number => 1, :bank_routing_number => 1)
=> starting SSL for x-test.chargify.com:443...
SSL established
<- "PUT /payment_profiles/3424529.xml
ActiveResource::ServerError: Failed.  Response code = 500.  Response message = Internal Server Error.
nickhammond commented 10 years ago

Going back to the original payment_profile portion of this issue. Is the intention from Chargify to add a way to figure out their primary payment profile and make it accessible via the API? Something like payment_type as an attribute on the subscription object? I noticed that payment_type is an attribute on the nested bank_account but not on credit_card.

jeremywrowe commented 10 years ago

@nickhammond I have a fix for this that I am about to push out here shortly.

nickhammond commented 10 years ago

@jeremywrowe awesome, thanks much.

nickhammond commented 10 years ago

@jeremywrowe Going back to your earlier comment. We can set payment_profile_attributes when setting up a subscription but if a subscription has a bank account as the primary payment profile then payment_profile currently returns nil.

irb(main):038:0> c.payment_profile
=> nil
irb(main):039:0> c.credit_card
NoMethodError: undefined method `credit_card' for #<Chargify::Subscription:0x007f9f02c19fc8>
        from /Users/nick/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/activeresource-3.2.13/lib/active_resource/base.rb:1470:in `method_missing'
        from (irb):39
        from /Users/nick/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/railties-3.2.13/lib/rails/commands/console.rb:47:in `start'
        from /Users/nick/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/railties-3.2.13/lib/rails/commands/console.rb:8:in `start'
        from /Users/nick/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/railties-3.2.13/lib/rails/commands.rb:41:in `<top (required)>'
        from script/rails:6:in `require'
        from script/rails:6:in `<main>'
irb(main):040:0> c.bank_account
=> #<Chargify::Subscription::BankAccount:0x007f9f02c56f18 @attributes={"bank_account_holder_type"=>"personal", "bank_account_type"=>"checking", "bank_name"=>"Wells", "billing_address"=>nil, "billing_address_2"=>nil, "billing_city"=>nil, "billing_country"=>nil, "billing_state"=>nil, "billing_zip"=>nil, "current_vault"=>"bogus", "customer_id"=>5354009, "customer_vault_token"=>nil, "first_name"=>"Diamond", "id"=>3705908, "last_name"=>"Dawes", "masked_bank_account_number"=>"XXXXXXX1", "masked_bank_routing_number"=>"XXXXXXX1", "vault_token"=>"1", "payment_type"=>"bank_account"}, @prefix_options={}, @persisted=false>