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

Credorax: Add 3DS 2.0 #3342

Merged
merged 1 commit into from
Sep 16, 2019
Merged

Credorax: Add 3DS 2.0 #3342

merged 1 commit into from
Sep 16, 2019

Conversation

nfarve
Copy link

@nfarve nfarve commented Sep 10, 2019

Adds 3DS 2.0 fields to credorax and a new field that is required for
passing 3DS authenticated fields.

Loaded suite test/remote/gateways/remote_credorax_test
..........................


26 tests, 73 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Loaded suite test/unit/gateways/credorax_test


23 tests, 124 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

@nfarve nfarve requested review from a team, britth and jeremywrowe September 10, 2019 19:14
Copy link
Contributor

@britth britth left a comment

Choose a reason for hiding this comment

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

had one question, but looks good!

@@ -270,13 +270,38 @@ def add_email(post, options)
def add_3d_secure(post, options)
if options[:eci] && options[:xid]
add_3d_secure_1_data(post, options)
elsif options[:execute_threed]
three_ds_2_options = options[:three_ds_2]
browser_info = three_ds_2_options[:browser_info]
Copy link
Contributor

Choose a reason for hiding this comment

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

should we check that there is a :three_ds_2 key in the options hash before proceeding in this part to prevent errors, maybe in the elsif add && options[:three_ds_2] or something similar

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah shouldn't this be options[:three_d_secure] , and condition on version? https://github.com/activemerchant/active_merchant/wiki/Standardized-3DS-Fields

Copy link
Author

Choose a reason for hiding this comment

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

@curiousepic We pass a different hash of values for 3DS direct.

Copy link
Author

Choose a reason for hiding this comment

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

three_ds_2_options = options[:three_ds_2]
browser_info = three_ds_2_options[:browser_info]
post[:'3ds_initiate'] = options[:three_ds_initiate] || '01'
post[:'3ds_purchasedate'] = Time.now.utc.strftime('%Y%m%d%I%M%S')
Copy link
Contributor

Choose a reason for hiding this comment

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

saw mention of something similar on another PR - not sure if there are any intricacies that need to be considered here in terms of Time.now v Time.current

Copy link
Author

Choose a reason for hiding this comment

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

Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Actually since I am using utc I think want Time.now actually.

Adds 3DS 2.0 fields to credorax and a new field that is required for
passing 3DS authenticated fields.

Loaded suite test/remote/gateways/remote_credorax_test
..........................

-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
26 tests, 73 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Loaded suite test/unit/gateways/credorax_test

-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
23 tests, 124 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@nfarve nfarve merged commit c9a851c into master Sep 16, 2019
@nfarve nfarve deleted the credorax_3ds2 branch September 16, 2019 20:36
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.

3 participants