-
-
Notifications
You must be signed in to change notification settings - Fork 76
Set source payment_type in cc hosted fields #79
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
class AddNotNullConstraintToSourcesPaymentType < ActiveRecord::Migration | ||
def change | ||
reversible do |dir| | ||
dir.up do | ||
SolidusPaypalBraintree::Source.where(payment_type: nil). | ||
update_all(payment_type: 'CreditCard') | ||
end | ||
end | ||
change_column_null(:solidus_paypal_braintree_sources, :payment_type, false) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ class Engine < Rails::Engine | |
|
||
initializer "register_solidus_paypal_braintree_gateway", after: "spree.register.payment_methods" do |app| | ||
app.config.spree.payment_methods << SolidusPaypalBraintree::Gateway | ||
Spree::PermittedAttributes.source_attributes << :nonce | ||
Spree::PermittedAttributes.source_attributes.concat [:nonce, :payment_type] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This value isn't whitelisted and can be modified by users. Further if we know the payment type in advance, we should probably avoid requiring it to be set in the form. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I need to whitelist it, otherwise I can not pass it into the controller from the view where the value needs top be set, as we don't know the type upfront.
We don't know the type upfront. I didn't find any code that guarantees this. Maybe you can point me to? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I meant that the value isn't whitelisted. I can insert "lol" into this field and it would save correctly. We should probably add an inclusion validation to the model to prevent this.
Once we have the token, As we only use this for display purposes, I no longer think that's worth doing though. |
||
end | ||
|
||
def self.activate | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,7 @@ | |
{ | ||
transaction: { | ||
nonce: "fake-valid-nonce", | ||
payment_type: "MonopolyMoney", | ||
payment_type: SolidusPaypalBraintree::Source::PAYPAL, | ||
phone: "1112223333", | ||
email: "[email protected]", | ||
address_attributes: { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
This isn't accurate. Is there a scenario that generated nil payment_types in the past?
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.
Yes. All credit card payments. PayPal and ApplePay payments have this value set via JS tailored params.
Although this isn't accurate it's the only thing we can do about, besides not doing this at all of course.
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.
Sure, fair point.