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

Vantiv(Litle): Add support for fraudFilterOverride #4544

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

rachelkirk
Copy link
Contributor

CER-51

This PR adds the fraudFilterOverride flag, a boolean which when set to true will override all fraud filters for the submitted transaction.

Unit Tests:
56 tests, 240 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote Tests:
56 tests, 226 assertions, 13 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
76.7857% passed

  • I ran the remote tests several times and 13 (sometimes 14) tests are failing and have been for over a year. I think this gateway is due for an overhaul but the failures aren’t related to the changes and are also failing on master.

Local Tests:
5288 tests, 76246 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

@rachelkirk rachelkirk requested a review from a team August 23, 2022 00:30
@@ -320,6 +320,7 @@ def add_auth_purchase_params(doc, money, payment_method, options)
add_merchant_data(doc, options)
add_debt_repayment(doc, options)
add_stored_credential_params(doc, options)
add_fraud_filter_override(doc, options)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it to auth_purchase_params but it may need to be added to echeck_purchase_params as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm...it's a little bit ambiguous from the Vantiv/WP docs, but what I see is that they call fraudFilterOverride a child element of either authorization or sale. Perhaps it depends on whether they consider echeckSale to be an entirely different thing than a sale or just a specific type of sale?

I think I would probably leave it as you have it here.

Copy link
Contributor

@jcreiff jcreiff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I confirmed that the remote tests are also failing on master. The newly added tests do pass.

@@ -320,6 +320,7 @@ def add_auth_purchase_params(doc, money, payment_method, options)
add_merchant_data(doc, options)
add_debt_repayment(doc, options)
add_stored_credential_params(doc, options)
add_fraud_filter_override(doc, options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm...it's a little bit ambiguous from the Vantiv/WP docs, but what I see is that they call fraudFilterOverride a child element of either authorization or sale. Perhaps it depends on whether they consider echeckSale to be an entirely different thing than a sale or just a specific type of sale?

I think I would probably leave it as you have it here.

CER-51

This PR adds the `fraudFilterOverride` flag, a boolean which when set to true will override all fraud filters for the submitted transaction.

Unit Tests:
56 tests, 240 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote Tests:
56 tests, 226 assertions, 13 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
76.7857% passed
* I ran the remote tests several times and 13 (sometimes 14) tests are failing and have been for over a year. I think this gateway is due for an overhaul but the failures aren’t related to the changes and are also failing on master.

Local Tests:
5288 tests, 76246 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
@rachelkirk rachelkirk force-pushed the CER-51_vantiv_fraudFilterOverride branch from 22f33de to 5b4cf95 Compare August 23, 2022 15:51
@rachelkirk rachelkirk merged commit 5b4cf95 into master Aug 23, 2022
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.

2 participants