spree_gateway icon indicating copy to clipboard operation
spree_gateway copied to clipboard

Published 20 hours ago • verified publisher icon spree

Accept an ActiveMerchant::Billing::Response when storing a credit card for Balanced

Open cwise opened this issue 12 years ago • 11 comments

Changed handling in create_profile for Balanced to accept an ActiveMerchant response with the card_uri and account_uri in authorization

Please see ActiveMerchant pull request: https://github.com/Shopify/active_merchant/pull/889

When this is pulled in to ActiveMerchant, my changes to Spree gateway can be pulled.

cwise avatar Oct 23 '13 18:10 cwise

I made a minor modification to this as the AM guys asked me to return the card_uri and account_uri in a different manner. I'll monitor my pull request on AM and will update this PR once a new AM gem is released.

cwise avatar Oct 25 '13 12:10 cwise

Ok an ActiveMerchant update has been released in 1.42.0 and forward that supports the credit card uri being returned in an ActiveMerchant response.

There doesn't appear to be an ActiveMerchant dependency in this gem. Should there be one? Or does that unnecessarily complicate things?

cwise avatar Nov 18 '13 19:11 cwise

Hey @cwise,

The ActiveMerchant dependency is listed within spree_core's gemspec because there are parts of Spree, such as the BogusGateway classes, which depend on ActiveMerchant.

I will be bumping the AM version later today. We are probably going to be doing a new Spree release (2.1.x, 2.0.x and maybe 1.3.x) sometime this week too.

radar avatar Nov 18 '13 20:11 radar

This is fixed with: 63dd56da5f16fa52e98bd77a1759339c892c3e4c

peterberkenbosch avatar Dec 06 '13 14:12 peterberkenbosch

Maybe I was to quick.. @cwise AM dependency has been updated. Does this PR needs any more work or can it be closed? //cc @radar

peterberkenbosch avatar Dec 06 '13 14:12 peterberkenbosch

Thanks Peter.

cwise avatar Dec 06 '13 15:12 cwise

Did the commit to handle the response get merged in? That's the bulk of the change, but it was tied to an update to AM that sends back a proper AM:Response instead of whatever it previously did.

cwise avatar Dec 06 '13 15:12 cwise

Ah, no that did not get merged in yet. I have 2 minor issues, it will not merge cleanly with master and I would love some regression tests as well. Could you look into that @cwise? Thanks!

peterberkenbosch avatar Dec 06 '13 15:12 peterberkenbosch

I'm unclear on the status of this. Has this been resolved or did it stall on specs?

JDutil avatar Oct 03 '14 18:10 JDutil

@cwise Can you rebase it against master & make sure everything's working like expected? Also - please provide some specs. Thanks in advance 👍

kushniryb avatar Jun 22 '18 10:06 kushniryb

Guys I'm soooooooo out of the weeds on this these days that it'd be tough for me to get involved. Let me check with our dev team on this and see where it was left.

cwise avatar Jun 22 '18 12:06 cwise