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

Authorize.NET: Update network token method #4852

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

almalee24
Copy link

Update network token payment method to clarify what is being sent.

Remote:
85 tests, 304 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed

Unit:
122 tests, 684 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed

@almalee24 almalee24 requested a review from a team August 11, 2023 19:45
@almalee24 almalee24 force-pushed the authorize_net_network_token branch 2 times, most recently from 885ae68 to 1c9e44f Compare August 23, 2023 14:41
Copy link
Contributor

@aenand aenand left a comment

Choose a reason for hiding this comment

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

This PR seems to remove any apple_pay flagging for AuthNet. Are we sure AuthNet does not care about any flagging to let them know if the payment_method is apple_pay or not?

@almalee24
Copy link
Author

This PR seems to remove any apple_pay flagging for AuthNet. Are we sure AuthNet does not care about any flagging to let them know if the payment_method is apple_pay or not?

Yes, I've been talking to them directly about this. The apple_pay flagging is only for the Accept Mobile SDK and customers have enabled Apple Pay from within the gateway that it’s tied to. Also by the way it's currently implement that part of the code isn't actually being used.

@almalee24 almalee24 requested a review from aenand August 23, 2023 16:44
end
end
end

def add_market_type_device_type(xml, payment, options)
return if payment.is_a?(String) || card_brand(payment) == 'check' || card_brand(payment) == 'apple_pay'
return unless payment.is_a?(CreditCard) && !payment.is_a?(NetworkTokenizationCreditCard)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return unless payment.is_a?(CreditCard) && !payment.is_a?(NetworkTokenizationCreditCard)
return if payment.is_a?(NetworkTokenizationCreditCard)

Is there a way to avoid the double negative here?

Copy link
Author

@almalee24 almalee24 Aug 25, 2023

Choose a reason for hiding this comment

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

I can split them in two different lines

Copy link
Contributor

@BritneyS BritneyS 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 to me! Thanks for diving deep into this one 🚀

Update network token payment method to clarify what is being
sent.

Remote:
85 tests, 304 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Unit:
122 tests, 684 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
@almalee24 almalee24 merged commit 544806e into master Oct 24, 2023
5 checks passed
@almalee24 almalee24 deleted the authorize_net_network_token branch October 24, 2023 14:48
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