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

Worldpay: Encyrpted ApplePay and GooglePay #5093

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

almalee24
Copy link

Add support for encrypted ApplePay and GooglePay.

@almalee24 almalee24 requested a review from a team April 15, 2024 19:13
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.

Is there a way to add a remote test?

@almalee24
Copy link
Author

Is there a way to add a remote test?

No unfortunately

@@ -981,7 +1000,15 @@ def payment_details(payment_method, options = {})
when String
token_type_and_details(payment_method)
else
type = network_token?(payment_method) || options[:wallet_type] == :google_pay ? :network_token : :credit
type = if network_token?(payment_method) || options[:wallet_type] == :google_pay
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to not have nested if statements here? Can we put another case for NetworkTokenizationCreditCard?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah let me fix this

Copy link
Author

Choose a reason for hiding this comment

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

I'll update it a bit but I can't change it to NetworkTokenizationCreditCard because network_token?(payment_method) was added by Shopify and I don't want to break an integration

@almalee24 almalee24 force-pushed the worldpay_encrypted_apple_google_pay branch from dac72a6 to e23e6ba Compare April 25, 2024 17:54
Comment on lines +1003 to +1011
type = if network_token?(payment_method)
payment_method.payment_data ? :encrypted_wallet : :network_token
else
options[:wallet_type] == :google_pay ? :network_token : :credit
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Do payment methods with options[:wallet_type] == :google_pay have payment_data? would this work?

Suggested change
type = if network_token?(payment_method)
payment_method.payment_data ? :encrypted_wallet : :network_token
else
options[:wallet_type] == :google_pay ? :network_token : :credit
end
type = if network_token?(payment_method) || options[:wallet_type] == :google_pay
payment_method.payment_data ? :encrypted_wallet : :network_token
else
:credit
end

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't. options[:wallet_type] == :google_pay is for a non-tokenized GooglePay which we treat as a CreditCard

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.

Code looks good to me! just an opinion on read-ability but whichever you choose works!

Add support for encrypted ApplePay and GooglePay.
@almalee24 almalee24 force-pushed the worldpay_encrypted_apple_google_pay branch from e23e6ba to 65fde2a Compare May 8, 2024 12:58
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.

None yet

2 participants