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

Adyen: add network_transaction_id to store call #4522

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Conversation

jcreiff
Copy link
Contributor

@jcreiff jcreiff commented Aug 3, 2022

This makes it possible to pass the networkTxReference field with store

CER-91

Unit:
95 tests, 482 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:
124 tests, 443 assertions, 3 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
97.5806% passed

One failing test is related to something that might be an outdated assumption (Adyen doesn’t allow recurring transactions with Cabal). The other two are 3DS2-related. All are failing on master.

Local:
5272 tests, 76166 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

@jcreiff jcreiff requested a review from a team August 3, 2022 21:00
@@ -993,6 +993,14 @@ def test_successful_tokenize_only_store
assert_equal 'Success', response.message
end

def test_successful_tokenize_only_store_with_ntid
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'm not sure if there are additional assertions that would make sense to add here...from what I could tell, it doesn't seem like Adyen's response really changes with the addition of this field to the request 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good, I wonder if you were to add the ntid to a different transaction type does it change those responses vs when it's not there? I don't know that that's relevant in terms of THIS test, but would maybe explain why it's not changing in the store response. Not asking you to go back and investigate that, just came to mind.

Copy link
Contributor

@rachelkirk rachelkirk left a comment

Choose a reason for hiding this comment

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

Tests pass (except the three remote test failures that are also failing on master) and looks good!

@@ -993,6 +993,14 @@ def test_successful_tokenize_only_store
assert_equal 'Success', response.message
end

def test_successful_tokenize_only_store_with_ntid
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good, I wonder if you were to add the ntid to a different transaction type does it change those responses vs when it's not there? I don't know that that's relevant in terms of THIS test, but would maybe explain why it's not changing in the store response. Not asking you to go back and investigate that, just came to mind.

This makes it possible to pass the networkTxReference field with `store`

CER-91
Unit:
95 tests, 482 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:
124 tests, 443 assertions, 3 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
97.5806% passed

One failing test is related to something that might be an outdated assumption (Adyen doesn’t allow recurring transactions with Cabal). The other two are 3DS2-related. All are failing on master.

Local:
5272 tests, 76166 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
@jcreiff jcreiff merged commit 7cbc1ee into master Aug 4, 2022
@jcreiff jcreiff deleted the CER-91_adyen_store branch September 14, 2022 15:02
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