Activemerchant 1.88 #21
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pretty much all remote tests fail, saying the card has been declined, with no additional information. I'm tracking that down, but I'm disinclined to hold based on that. Unit: 19 tests, 89 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 19 tests, 27 assertions, 10 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 47.3684% passed Closes activemerchant#2889
Scrub the 3DS `xid` and `cavv` from the transcript. Unit Test: 5 tests, 17 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote Test (1 unrelated error: due old `offsite_purchase` test setup): 13 tests, 39 assertions, 0 failures, 1 errors, 0 pendings, 0 omissions, 0 notifications 92.3077% passed Closes activemerchant#2771
This strictly speaking should not be necessary, but we're seeing enough of this happening that it seems worth doing (and is otherwise harmless). Unit: 18 tests, 61 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 18 tests, 45 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Closes activemerchant#2890
Closes activemerchant#2891 Remote: 34 tests, 86 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Unit: 21 tests, 97 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
Adds ACH capabilities to Moneris US Gateway One failing test for CVV verification that are unrelated to these changes. Loaded suite test/remote/gateways/remote_moneris_us_test Failure: test_cvv_no_match_when_enabled(MonerisUsRemoteTest) 30 tests, 118 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 96.6667% passed Loaded suite test/unit/gateways/moneris_us_test 33 tests, 164 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Closes activemerchant#2888
Closes activemerchant#2895 Remote (7 failures unrelated to changes verified on master): 21 tests, 103 assertions, 7 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 66.6667% passed Unit: 21 tests, 445 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
Unit: 21 tests, 89 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 23 tests, 54 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Closes activemerchant#2896
- Move the recording of the request start time to the beginning of `Connection#request`. This ensures that everything is instrumented and assigns the variable used in the `ensure` block as soon as possible before any possible exceptions.
- Copy the headers argument object in `Connection#request` to prevent mutation of an object that could be frozen. - Add a test for `PostsData#raw_ssl_request` asserting against headers argument mutation to protect against mutation happening unknowingly at that level. Closes activemerchant#2892
Unit: 20 tests, 67 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 20 tests, 52 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Closes activemerchant#2898
Previously, Paymentez only supported capturing the original amount. It now supports capturing any amount the processor is okay with--by default, 100% or more of the amount specified. As part of this commit, I've repaired all remote tests, plus altered the partial cpature test, since it's now explicitly documented that the test, as written, should fail (which it does). Unit: 16 tests, 48 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 16 tests, 41 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Closes activemerchant#2900
Unit: 6 tests, 6 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Closes activemerchant#2571 Closes activemerchant#2217
Several additional settings were introduced to work around Travis bug 8978, but that has since been resolved. Additionally, also remove a fully redundant `script` line.
This introduces RuboCop to Active Merchant, defaulting to the normal Ruby community guidelines. To grandfather in our existing code, I've pre-generated a `.rubocop_todo.yml` file, which will allow gradual migration. To run RuboCop, you can run `rake rubocop`, but I've also introduced a test:local Rake target that runs both the unit tests and RuboCop checks. I've made this be the default Rake task as well, which means Travis will (if this PR lands) require RuboCop passing for test approval. As a proof-of-concept, I went ahead and fixed WhenThen and YodaConditional style violations, which had sufficiently few violations to work for demo purposes. All PayPal-related code is excluded because it crashes RuboCop. Unit: 3864 tests, 67919 assertions, 0 failures, 0 errors, 0 pendings, 2 omissions, 0 notifications 100% passed
This isn't necessary locally, but breaks Travis
The following style rules are no longer exempted: - Layout/AccessModifierIndentation - Layout/AlignArray - Layout/BlockAlignment - Style/TrivialAccessors - Style/UnlessElse - Style/UnneededConditional These all had only a few violations each, so I've folded them all into a single commit. I also regenerated .rubocop_todo.yml to reflect the new error counts, since some of these fixes had meaningfully positive impacts on other violations. Unit: 3874 tests, 67956 assertions, 0 failures, 0 errors, 0 pendings, 2 omissions, 0 notifications 100% passed
When transactions are not yet settled a refundSingleTransaction needs to be called. This tries to use this method if a cancelDeposit fails. If both calls are made the messages from both transactions are captured. . Loaded suite test/unit/gateways/visanet_peru_test ............. 13 tests, 67 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Loaded suite test/remote/gateways/remote_visanet_peru_test ................. 17 tests, 56 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
None of these resulted in build or test failure, but all are flagged directly on a test:unit run. The specific issues flagged were: active_merchant/lib/active_merchant/billing/gateways/mundipagg.rb:69: warning: assigned but unused variable - post active_merchant/lib/active_merchant/billing/gateways/litle.rb:81: warning: assigned but unused variable - kind active_merchant/lib/active_merchant/billing/gateways/litle.rb:195: warning: assigned but unused variable - transaction_id active_merchant/lib/active_merchant/billing/gateways/borgun.rb:99: warning: assigned but unused variable - batch active_merchant/test/unit/gateways/card_connect_test.rb:186: warning: assigned but unused variable - response active_merchant/test/unit/gateways/checkout_v2_test.rb:219: warning: ambiguous first argument; put parentheses or a space even after `/' operator active_merchant/test/unit/gateways/credorax_test.rb:222: warning: ambiguous first argument; put parentheses or a space even after `/' operator active_merchant/test/unit/gateways/optimal_payment_test.rb:445: warning: mismatched indentations at 'end' with 'def' at 441 None of these are related to RuboCop. 3874 tests, 67956 assertions, 0 failures, 0 errors, 0 pendings, 2 omissions, 0 notifications 100% passed
iDeal is a Dutch offsite payment method and belongs in https://github.com/activemerchant/offsite_payments
Yes, I cheated and used rubocop to help with this, but I also did
actually audit the result, and it's pretty clean to read through with a
good diff tool since no spacing, etc. was changed. My recommendation
for review would be to look for and notice the absence of `#{}` in any
of the changed strings, meaning that the `"` to `'` conversion is safe.
Unit: 3874 tests, 67956 assertions, 0 failures, 0 errors, 0 pendings, 2 omissions, 0 notifications
100% passed
Also, options were not previously being passed to the commit method, so the metadata header was not being utilized until these changes. Remote: 15 tests, 57 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Unit: 13 tests, 84 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
Ruby supports setting this from version 2.5, I backported this for 2.3 and 2.4 with the min_max_ssl gem. For more details about the deadline: https://blog.pcisecuritystandards.org/are-you-ready-for-30-june-2018-sayin-goodbye-to-ssl-early-tls
The testing URL for Payment Express is different from the production URL. Add the test URL, and properly toggle based on the testing flag. For more details, see https://www.paymentexpress.com/developer-e-commerce-paymentexpress-hosted-pxpay Closes activemerchant#2231
Unit: 22 tests, 110 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 23 tests, 70 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
This is actually two fixes, Layout/SpaceInsideBlockBraces and Layout/SpaceBeforeBlockBraces, because the diff actually looks worse if you do either of them alone (and the same lines largely get touched on the follow-up diff anyway).
Forte uses different transaction id's for captures. When trying to void a capture this results in a error. This saves the original auth transaction id and authorization id in the capture authorization so the original auth will be voided or refunded. Loaded suite test/unit/gateways/forte_test 20 tests, 67 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Loaded suite test/remote/gateways/remote_forte_test 21 tests, 59 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Previously, on Battlestar Galactica, we only filtered out the address if the fields were nil. But some of our library consumers like to pass in empty strings, which we treated as actual data, whereas Braintree Blue weirdly insists "" is not a valid address. Treat empty strings the same as nil values. Unit: 57 tests, 150 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 66 tests, 378 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
- During every ActiveSupport major bump, this gemspec needs to be updated, I think we are being too conservative and having an upperbound limit doesn't help. If we wanted to be really conservative we would have to add a upperbound constraint on the minor version as well (`< 5.x`) since Rails doesn't follow semver and minor version can introduce breaking changes. What I think is better is to remove the upperbound constraint, and test AM directly on ActiveSupport edge. Looking at the commit rates on this project, CI would run largely enough to cover us. But we can also add another safety net and run travis on a schedule.
MasterCard and Visa credits do not work the same way, so we were erroneously counting MasterCard credits as failed. Count them as successful. Unit: 40 tests, 227 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 28 tests, 111 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
Unit: 41 tests, 231 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
Refunds for bank accounts require a different structure than credit cards. The routing number, account number and account type must also be passed in the request. Loaded suite test/unit/gateways/authorize_net_test ................................................................................................ 96 tests, 565 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Loaded suite test/remote/gateways/remote_authorize_net_test ..................................................................... 69 tests, 238 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
Orbital merchant IDs were historically strings, but can now also be integers. To avoid any excitement, coerce them to strings before use. Unit: 70 tests, 422 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed 23 tests, 144 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Closes activemerchant#3072
Unit: 130 tests, 698 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 67 tests, 313 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Closes activemerchant#3057
TrustCommerce supports ach transactions for purchase and credit. This also adds the `aggregator_id` which will become mandatory. Loaded suite test/remote/gateways/remote_trust_commerce_test ....... 15 tests, 59 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Loaded suite test/unit/gateways/trust_commerce_test ......... 9 tests, 22 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
Payeezy allows $0 authorization. Updating from $1 verify to $0 to resolve failures for customer. ENE-59 Closes activemerchant#3074 Unit Tests: 33 tests, 156 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote Tests: 33 tests, 131 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
ENE-64 Unit: 10 tests, 34 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 16 tests, 56 assertions, 5 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 68.75% passed - Failures existing on earlier commits, not related to change made in this commit.
Port changes from pr 18 commit 954d94a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Syncs upstream activemerchant into Veracross activemerchant.