Consistent #credit API
Nathaniel Talbott
co-founders of http://spreedly.com. Since my work at Spreedly involves
working with so many different gateways - I've added one gateway from
scratch so far, and done the work to merge a couple more - the Shopify
folks invited me to dig in and help out with the ongoing work on
ActiveMerchant. At this point my focus is pretty much exclusively on
Gateways; I haven't done anything with Integrations to date.
As part of rolling some of our work at Spreedly back in to trunk, I
want to get some opinions on how to clean up the #credit API across
the various gateways. By way of background, there are two types of
crediting that gateways support: referenced and general. A referenced
credit allows the return of some or all of the funds for a specific
transaction, while a general credit allows putting an arbitrary amount
of funds on to a payment source regardless of whether there are any
transactions against it. Referenced credits are far and away the most
common type supported by gateways, but some gateways support general
credits as well.
Most of the gateways that support both referenced and general credits
simply provide the #credit method, and if the payment source passed in
is a String they do a referenced credit, otherwise they assume it's a
full payment method and do a general credit. This is OK, but I'm not
real fond of switching on the type of the argument provided (more on
that below).
The real problem arises in that the SmartPsGateway (subclassed by
BraintreeOrangeGateway and TransaxGateway) and the
BraintreeBlueGateway both provide two methods for these two types of
credits: #refund and #credit. #refund does a referenced credit, and
#credit does a general credit. This breaks the API expectations for
the #credit method since the most common behavior for #credit is a
referenced credit.
Here are a few options I've come up with to handle these two types of
#credit more consistently:
1. Do nothing, leaving these three gateways with different API behavior.
Pro: doesn't break those already implemented against these three gateways.
Con: means anyone switching from another gateway to these gateways
will get a surprise when they try to do a referenced #credit as per
the usual API expectation.
Con: requires implementors that credit against a lot of gateways (i.e.
Shopify, Spreedly, etc.) to special case these three gateways.
2. Switch these three gateways to only have #credit, and if the
payment source is a string, do a referenced credit, otherwise do a
general credit.
Pro: only impacts these three gateways.
Con: Braintree also allows doing a general credit against a vaulted
card, and that won't work any longer since the vault id is a String.
Con: switching behavior based on parameter type seems like a bad
long-term design decision (see previous Con).
Con: will break those who have already implemented any of these three gateways.
3. Provide one #credit method, and choose reference vs. general based
on an option parameter. A reference credit is the default, but passing
:general => true does a general credit.
Pro: makes referenced credit vs. general credit explicit.
Pro: unchanged API for the most common case (reference credit).
Con: breaks the API for those doing general credits (could mitigate by
still having a parameter type check with a deprecation warning).
Con: introduces a "magic option" to the #credit API.
Would love to hear feedback on these options, as well as any
additional options you see. Thanks!
--
Nathaniel Talbott
<:((><
Cody Fauser
Shopify isn't using #credit or #refund at all yet, which is probably
why they ended up being inconsistent in the first place. We plan to
add support for crediting and refunding, so it would be great to
resolve this.
It seems like having #refund and #credit is a good split and
accurately describes what is happening in each use case. We could go
with that convention, as it allows credit card vault references to
continue working.
A plan of attack could be:
1. Add #refund to all gateways that support reference credits
2. Add deprecation warnings to those gateways in their credit method
when a string reference is used
3. Remove support for reference strings in credit in version 2.0
Thoughts?
> --
> You received this message because you are subscribed to the Google Groups "Active Merchant" group.
> To post to this group, send email to activem...@googlegroups.com.
> To unsubscribe from this group, send email to activemerchan...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/activemerchant?hl=en.
>
>
--
Cody Fauser
http://shopify.com - e-commerce done right
http://www.codyfauser.com - blog
http://peepcode.com/products/activemerchant-pdf - ActiveMerchant PeepCode
Benjamin Curtis
--
Benjamin Curtis
http://www.uploadjuicer.com/ - Hosted image resizing
http://catchthebest.com/ - Team-powered recruiting
http://www.bencurtis.com/ - Personal blog
Nathaniel Talbott
> It seems like having #refund and #credit is a good split and
> accurately describes what is happening in each use case. We could go
> with that convention, as it allows credit card vault references to
> continue working.
>
> A plan of attack could be:
>
> 1. Add #refund to all gateways that support reference credits
> 2. Add deprecation warnings to those gateways in their credit method
> when a string reference is used
> 3. Remove support for reference strings in credit in version 2.0
I think that would provide a good consistent API. The two cons I see are:
- most gateways will end up with only a #refund method and no #credit
method (with the latter throwing a deprecation warning until 2.0),
which is a very wide-reaching API change that will effect 50%+ of
existing gateway classes.
- this is a significant mismatch from the terminology used by the vast
majority of gateways; "credit" is far and away the most common
term/action used for credits of any kind (and reference credits are
the most common type supported; general credit support is pretty
rare).
I'm ambivalent about this approach in general; I like splitting up
#credit/#refund in one sense, but on the flipside general credit
capabilities are rare enough I'm not sure if they warrant their own
method or if they should just be an option on #credit.
Since I'm ambivalent I'll leave the call up to ya'll; if you're OK
with these cons then I can spearhead making the change. Does
ActiveMerchant have a preferred method for marking/warning about
deprecated API's?
--
Nathaniel Talbott
<:((><