-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
Backport HostedForm device data collection to v2.x #135
Conversation
Offenses: app/models/solidus_braintree/transaction.rb:20:21: C: [Correctable] Style/InverseMethods: Use invalid? instead of inverting valid?. if address && !address.valid? ^^^^^^^^^^^^^^^ app/models/solidus_braintree/transaction_import.rb:12:57: C: [Correctable] Style/InverseMethods: Use invalid? instead of inverting valid?. errors.add("Address", "is invalid") if address && !address.valid? ^^^^^^^^^^^^^^^ app/models/solidus_braintree/transaction_import.rb:14:10: C: [Correctable] Style/InverseMethods: Use invalid? instead of inverting valid?. if !transaction.valid? ^^^^^^^^^^^^^^^^^^^
Offenses: app/models/solidus_braintree/transaction.rb:20:10: C: [Correctable] Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method. if address && address.invalid? ^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/solidus_braintree/transaction_import.rb:12:46: C: [Correctable] Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method. errors.add("Address", "is invalid") if address && address.invalid? ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Closes #115. This commit adds device data collection. See: https://developer.paypal.com/braintree/docs/guides/premium-fraud-management-tools/device-data-collection
…sBraintree hosted form Closes #126.
@@ -17,7 +17,7 @@ class Transaction | |||
unless payment_method.is_a? SolidusBraintree::Gateway | |||
errors.add(:payment_method, 'Must be braintree') | |||
end | |||
if address && !address.valid? | |||
if address&.invalid? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these changes to fix the RuboCop offenses in CI.
@@ -9,9 +9,9 @@ class InvalidImportError < StandardError; end | |||
include ActiveModel::Model | |||
|
|||
validate do | |||
errors.add("Address", "is invalid") if address && !address.valid? | |||
errors.add("Address", "is invalid") if address&.invalid? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these changes to fix the RuboCop offenses in CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, but I have a doubt: is this supposed to be part of 2.0? I thought we wanted to leave the 2.0 version as identical as possible to SolidusPaypalBraintree to facilitate the upgrade. If we add other features, maybe it will be more complex to upgrade.
It's not the end of the world but we can release this change in a 2.1 version. WDYT?
@kennyadsl Makes sense. Plus, I think it would be ideal if we sort out the remaining concerns with device data (compatibility with other payment methods, reuse with saved payment sources) before we backport it to 2.0x. I'm closing this for now. We can reopen it once we're ready. |
Summary
Closes #132.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: