one to one relationship for gateway customer #212
Conversation
app/controllers/solidus_paypal_braintree/client_tokens_controller.rb
Outdated
Show resolved
Hide resolved
| @@ -375,5 +391,16 @@ def customer_profile_params(payment) | |||
|
|
|||
| params | |||
| end | |||
|
|
|||
| def payment_method_params(payment) | |||
There was a problem hiding this comment.
This method smells like it shouldn't be here, what about a separate class? Anyway I'd call this method payment_method_attributes_from(payment) at least or payment_to_braintree_payment_method_attributes(payment), not sure what's best, or worst? 🙂
There was a problem hiding this comment.
I agree with this, I followed suit with the customer_profile_params(payment) method for consistency.
|
@kennyadsl The reason all the VCR cassettes changed is because there is now a different api call that is made. When there is a braintree customer attached to a spree user then we This is one of the reasons I'm not a fan of VCR (there are many). VCR is very quick and simple but can create issues over time. I would love to open a PR to remove VCR altogether and stub the service with sinatra. As far as commits go, I'm probably going to squash all of this into 1 commit |
skukx
force-pushed
the
206-one-to-one-relationship-for-gateway-customer
branch
5 times, most recently
from
b683d3c
to
084e5de
Compare
Ensure that when creating braintree customers we create one per spree user. To do this, first generate a token with the given customer id if available. When a braintree customer already exists, rather than create a new braintree customer, add the payment method to the existing braintree customer.
skukx
force-pushed
the
206-one-to-one-relationship-for-gateway-customer
branch
from
084e5de
to
0f8e967
Compare
| end | ||
|
|
||
| def customer_id | ||
| return unless try_spree_current_user&.braintree_customer |
There was a problem hiding this comment.
There is a bug with the ClientTokensController. The controller generates the token for the current user. However if that endpoint is called from admin then it will attach the payment source to the administrating user rather than the user on the order
There was a problem hiding this comment.
Debating if the token should be generated when the view is rendered. Maybe as a hidden input
There was a problem hiding this comment.
Not sure, what's the difference?
There was a problem hiding this comment.
See implementation 2 here: solidusio/solidus_braintree#79
The main difference is that the view no longer makes an api request to obtain the client token but rather just generates it when the view renders. The api request was only made if the new braintree payment method was selected (javascript events). This change generates a token even if the user may not be adding a new payment method. Therefore it could mean a slight increase in api requests sent to braintree. I think this should be negligible though
There was a problem hiding this comment.
What about who is using this gem without a Rails frontend, eg on React or Vue.js? I think we still need this controller to work for them, right?
|
@skukx Hello again! Now that the other PR has been merged, would you mind rebasing this one as well? Thanks! |
Ensure that when creating braintree customers we create one per spree
user. To do this, first generate a token with the given customer id if
available. When a braintree customer already exists, rather than create
a new braintree customer, add the payment method to the existing
braintree customer.
Closes #206