ProPay: Add gateway #2405

davidsantoso · 2017-04-26T18:12:54Z

Adding the ProPay gateway. Just to note- still waiting on the production URL. I'll update the PR once it's given before merging in.

Remote Test Run

➜ ruby -Itest test/remote/gateways/remote_pro_pay_test.rb
Loaded suite test/remote/gateways/remote_pro_pay_test
Started
...................

Finished in 58.230136 seconds.
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
19 tests, 49 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
0.33 tests/s, 0.84 assertions/s

Unit Test Run

➜ ruby -Itest test/unit/gateways/pro_pay_test.rb
Loaded suite test/unit/gateways/pro_pay_test
Started
................

Finished in 0.069866 seconds.
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
16 tests, 68 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
229.01 tests/s, 973.29 assertions/s

curiousepic · 2017-04-27T15:12:40Z

lib/active_merchant/billing/gateways/pro_pay.rb

+      self.money_format = :cents
+      self.supported_cardtypes = [:visa, :master, :american_express, :discover]
+
+      self.homepage_url = 'http://www.example.net/'


https://www.propay.com ?

curiousepic · 2017-04-27T15:17:55Z

lib/active_merchant/billing/gateways/pro_pay.rb

+      def refund(money, authorization, options={})
+        request = build_xml_request do |xml|
+          xml.transNum authorization
+          xml.amount money if money


Should this (and the one in capture) be amount(money)?

It should! Good catch.

curiousepic · 2017-04-27T15:21:58Z

lib/active_merchant/billing/gateways/pro_pay.rb

+      def capture(money, authorization, options={})
+        request = build_xml_request do |xml|
+          xml.amount money if money
+          xml.transNum authorization


Not sure how I feel about having both these direct additions to the xml in the action methods, and the one-line add_account and add_recurring methods extracted. Not a big deal though.

Hmmm. Sometimes I think it's ok to have the method just add the single value instead of always using any extracted methods. But, the more I think about it, it's probably better for me to keep using to extracted methods.

curiousepic · 2017-04-27T15:23:07Z

lib/active_merchant/billing/gateways/pro_pay.rb

+
+      def verify(credit_card, options={})
+        MultiResponse.run(:use_first_response) do |r|
+          r.process { authorize(100, credit_card, options) }


Doesn't accept 0-value authorizes?

Hmmm I didn't try it actually. I've actually always gone off of the 100 that comes with the generator. I'll give it a shot and see what happens.

Just tried 0 auth during verify and got an Invalid amount error. 😿

curiousepic · 2017-04-27T15:36:21Z

lib/active_merchant/billing/gateways/pro_pay.rb

+      end
+
+      def add_recurring(xml, options)
+        xml.recurringPayment options[:recurring_payment]


Any best practice for options passed to a gateway that are boolean being true/false vs the value of what's actually passed to the gateway? Or are there more options than Y/N?

I've personally always thought it's better to pass the same value as what's in the gateway docs. Seems more explicit to me instead of relying on the underly adapter to do the right thing depending on true/false. That being said, I don't think there's actually a convention around this yet...

I think that does make sense; I'll try to remember to follow that in the future.

davidsantoso · 2017-04-27T19:11:12Z

Alrighty, updates made. Thanks for the second set of eyes @curiousepic. Just waiting on the production URL from ProPay now...

davidsantoso · 2017-04-27T20:47:47Z

Live URL added. Just waiting on a 👍!

curiousepic · 2017-04-28T14:39:14Z

@shasum Want to give a once-over and 👍 ?

curiousepic

LGTM!

shasum

Everything looks good 👍

shasum · 2017-04-27T18:13:47Z

lib/active_merchant/billing/gateways/pro_pay.rb

+  module Billing #:nodoc:
+    class ProPayGateway < Gateway
+      self.test_url = 'https://xmltest.propay.com/API/PropayAPI.aspx'
+      self.live_url = 'https://example.com/live'


Also this one, the live_url? :)

shasum · 2017-04-27T18:20:37Z

lib/active_merchant/billing/gateways/pro_pay.rb

+        request = build_xml_request do |xml|
+          xml.transNum authorization
+          xml.amount money if money
+          xml.currencyCode if options[:currency]


Not sure if it's needed. Noticed capture does not have currency code unlike refund.

shasum · 2017-04-27T18:24:47Z

lib/active_merchant/billing/gateways/pro_pay.rb

+      end
+
+      def add_account(xml, options)
+        xml.accountNum options[:account_num]


Is account_num likely to change per transaction. Just want to make sure it's better as an option vs a credential.

@shasum great question! Yes it is. ProPay is a payment gateway for platforms so for each transaction you need to specify the merchant (on your platform) that you want to actually receive the funds. Similar to WePay.

davidsantoso · 2017-04-28T14:47:10Z

Thanks @shasum and @curiousepic!

Closes activemerchant#2405

davidsantoso self-assigned this Apr 26, 2017

davidsantoso requested review from shasum and curiousepic April 26, 2017 18:12

davidsantoso force-pushed the propay branch 2 times, most recently from 8ff149f to ea9f955 Compare April 26, 2017 19:48

curiousepic reviewed Apr 27, 2017

View reviewed changes

davidsantoso force-pushed the propay branch from ea9f955 to e5cfea3 Compare April 27, 2017 19:08

ProPay: Add gateway

b479bda

davidsantoso force-pushed the propay branch from e5cfea3 to b479bda Compare April 27, 2017 20:47

curiousepic approved these changes Apr 28, 2017

View reviewed changes

shasum approved these changes Apr 28, 2017

View reviewed changes

davidsantoso closed this in edaef2d Apr 28, 2017

asanger pushed a commit to asanger/active_merchant that referenced this pull request Jun 6, 2017

ProPay: Add gateway support

1ff5cb0

Closes activemerchant#2405

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ProPay: Add gateway #2405

ProPay: Add gateway #2405

davidsantoso commented Apr 26, 2017 •

edited

curiousepic Apr 27, 2017

davidsantoso Apr 27, 2017

curiousepic Apr 27, 2017 •

edited

davidsantoso Apr 27, 2017

curiousepic Apr 27, 2017

davidsantoso Apr 27, 2017

curiousepic Apr 27, 2017

davidsantoso Apr 27, 2017

davidsantoso Apr 27, 2017

curiousepic Apr 27, 2017

davidsantoso Apr 27, 2017

curiousepic Apr 27, 2017

davidsantoso commented Apr 27, 2017

davidsantoso commented Apr 27, 2017

curiousepic commented Apr 28, 2017

curiousepic left a comment

shasum left a comment

shasum Apr 27, 2017

shasum Apr 27, 2017

shasum Apr 27, 2017

davidsantoso Apr 28, 2017

shasum Apr 28, 2017

davidsantoso commented Apr 28, 2017

ProPay: Add gateway #2405

ProPay: Add gateway #2405

Conversation

davidsantoso commented Apr 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

curiousepic Apr 27, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidsantoso commented Apr 27, 2017

davidsantoso commented Apr 27, 2017

curiousepic commented Apr 28, 2017

curiousepic left a comment

Choose a reason for hiding this comment

shasum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidsantoso commented Apr 28, 2017

davidsantoso commented Apr 26, 2017 •

edited

curiousepic Apr 27, 2017 •

edited