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

iATS Payments gateway support #1133

Closed
wants to merge 1 commit into from
Closed

Conversation

rwdaigle
Copy link
Contributor

This commit adds support for the purchase and refund operations on the iATS Payments gateway. This replaces the existing iATS implementation (http://cl.ly/V8DE) and supersedes another, existing, pull request (http://cl.ly/V88q).

There is a backwards-incompatible change which I'd like advice on how to handle: The original implementation used credentials named login and password whereas this implementation sticks with gateway-specific naming of agent_code and password, and requires an additional region option. I don't believe the current implementation is being used, but do want to know how such changes should be handled.

@ntalbott
Copy link
Contributor

@rwdaigle ActiveMerchant has a deprecation facility; easiest to observe by grepping for assert_deprecation_warning. I'd recommend still accepting :login but dropping a deprecation warning that indicates the switch to :agent_code; doing so also flags the code for later removal.

As for the new :region parameter, I'd recommend simply having a default for it. Looks like formerly the na url was always used, so defaulting to that should allow anyone using the adapter today to keep on trucking.

This commit adds support for the `purchase` and `refund` operations
on the iATS Payments gateway. This replaces the existing iATS
implementation (http://cl.ly/V8DE) and supersedes another, existing,
pull request (http://cl.ly/V88q).
@rwdaigle
Copy link
Contributor Author

@ntalbott ntalbott closed this in 3f84370 Apr 23, 2014
@ntalbott
Copy link
Contributor

Merged.

@stephenfb
Copy link

Hi - thanks very much for merging this!
Two quick things

  1. the region should not default to NA - the big advancement to this new merge was the ability for USD and CAD currencies to go through the NA server, and the rest of the world - GBP, Euro etc (180 other countries) to go through our UK server. By defaulting to NA, you exclude the 180 other countries and we have a number of UK/Euro/Francs customers waiting to use Shopify with iATS.
    Could you make change back for CAD and USD to go through NA server, but everything else to go through UK server? All functions (charge, refunds etc) are the same, just the server changes.
  2. Refunds dont seem to work - when I select refund on an order the box is empty:

2014-04-28_1753

@ntalbott
Copy link
Contributor

@stephenbiats in the original patch (#977) the determination of region was done based on the credit card's country, which is an error-prone check that's really outside the scope of ActiveMerchant, and often not possible since country is not always supplied with credit card data.

That said, you seem to be implying now that the determination should be made based on currency, which I think we could do within ActiveMerchant. Please confirm: can the region be determined solely based on USD/CAD vs. other currencies?

@ntalbott
Copy link
Contributor

@stephenbiats RE refunds, that is a Shopify issue, not an ActiveMerchant issue, so you should take it up with Shopify's payments team directly.

@stephenfb
Copy link

@ntalbott You are correct, the cr card country is not the correct determination of the region.
The region is determined by the currency of the store. In other words if the currency of the store is CAD or USD - then the NA server should be used. If the currency of the store is GBP, Euro etc (basically anything else) - then the UK server should be used.
This is because iATS links the merchant account to the currency of the bank account where the funds need to be deposited. So for example GBP sales will go to a GBP merchant account and GBP bank account via the UK server, USD sales will go to USD merchant account and USD bank account via the NA server.

@ntalbott
Copy link
Contributor

@stephenbiats we've discussed this more internally here at Spreedly, and I've got another question for you: will the same set of merchant credentials ever need to interact with both urls? Or will a given merchant always be interacting with one of the urls?

@bslobodin
Copy link
Contributor

I'd be interested to know as well, it may be that we could use class IatsPaymentsUKGateway < IatsPaymentsGateway with a distinct service_url?

@stephenfb
Copy link

@ntalbott No, the merchant credentials are unique to a currency - so there will not be a case where the same set of merchant credentials will interact with both URL's. If a client wants a USD store and funds deposited in USD - they will get one set of merchant credentials, and another set if they have a GBP store and funds deposited into GBP account.
What may be a request is that a client may want a USA store and a GBP store with funds going into 2 different accounts. In that case they would need the ability to store 2 merchant credentials and link the correct product price to the correct merchant credential - or they would need to open 2 Shopify stores.
I dont think Shopify can manage multiple payment gateways in one Shop, so it seems they would need to open 2 Shopify stores if they wanted to process in different currencies.

@ntalbott
Copy link
Contributor

@stephenbiats based on that I think the current implementation is correct: the region should be specified as one of the credentials. In Spreedly for instance the region is always required when setting up a gateway: https://docs.spreedly.com/payment-gateways/iats-payments.

It seems like the real issue is not the adapter, but rather that Shopify should expose the region as a configuration value; maybe @bslobodin can help with that?

The other option would be for iATS to fix this on their side, since it really seems like one URL should be provided and then iATS should be making the determination internally as to how to route the request. But if this must be pushed down into the client, I think it has to be exposed to the merchant as a gateway setup option.

@stephenfb
Copy link

@ntalbott I think the best solution at this stage is for iATS to have 2 services in AM - iATS NA (for all NA transactions) and iATS EU (for all transactions outside NA). Then a client just needs to choose the correct one based on where they are and what iATS account they have. Could this work?

@ntalbott
Copy link
Contributor

@stephenbiats that's exactly the effect of specifying the region with the credentials; the code needs no changes to support that approach. Of course users of the code such as Shopify are free to present the selection of the region however they'd like, including making it look as though two different gateways are being picked between, but you should take that up with them directly.

@bslobodin
Copy link
Contributor

I like the distinct gateways idea, mostly because our configuration options are currently limited to text fields and it would be awkward to have to ask merchants to type something like NA or EU in. It would be much simpler for us to present iATS Americas and iATS Europe as distinct gateways, but to support it, we'd need two distinct gateways in AM. Implementation wise, one would be a subclass of another, overriding the URL, I suppose. @ntalbott would that work for you guys?

@ntalbott
Copy link
Contributor

@bslobodin I'm very much against creating gateway subclasses simply for presentation purposes - I think it clutters up the AM codebase, and would like to actually go through and eliminate all the gateway subclasses that don't add or modify behavior in the future.

I know that in the Spreedly codebase we have our own wrappers around the ActiveMerchant gateways which allow us to use the same AM gateway class for two different "presented" gateways if we choose - perhaps something like that would work for Shopify?

@bslobodin
Copy link
Contributor

Ya, let's leave it as is, I'll see what I can do.

@stephenfb
Copy link

Please let me know if there is anything you would like me to do to help. Can create 2 AM bases, rename (NA and Euro) and do pull request if like?

@bslobodin
Copy link
Contributor

Nope, I already made a change on our end. In Shopify there will be 2 options iATS (North America) and iATS (Europe), both will use existing gateway and set proper region.

@stephenfb
Copy link

FANTASTIC!!! Thank you! Please let me know when done and I will go in an test.

@bslobodin
Copy link
Contributor

@stephenbiats will do, probably will get deployed by the end of the week. In the meantime, it'll just be defaulting to 'na'.

@ntalbott
Copy link
Contributor

@bslobodin 🎆

@stephenfb
Copy link

@bslobodin hi, any news on this, did you manage to get it done this week?

@bslobodin
Copy link
Contributor

Not quite. Turns out, we have a freeze on a specific component undergoing
major refactoring at the moment, so splitting iATS into two regions will
have to wait until that is complete. I'll keep you updated.

@stephenfb
Copy link

Thank you!

@stephenfb
Copy link

@bslobodin Hi, any news on this?

@bslobodin
Copy link
Contributor

@stephenbiats it may be a bit still.

@stephenfb
Copy link

Hi - just following up - any news on this yet?

@stephenfb
Copy link

@bslobodin @ntalbott Hi, any news on this? Has this been completed yet? Please let me know if there is anything need to do my side. Have more EU customers keen to use Shopify and iATS and waiting for this release.

@ntalbott
Copy link
Contributor

@stephenbiats this is out of my bailiwick now; I think @odorcicd is the person you'll need to take Shopify issues up with these days.

@stephenfb
Copy link

Thanks @ntalbott. @odorcicd @bslobodin can you help please?

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.

None yet

4 participants