From c50ece2efcc1d3f74e926b92831bf1ea941dfa1a Mon Sep 17 00:00:00 2001 From: Russell Osborne Date: Tue, 30 Jan 2018 16:22:54 -0500 Subject: [PATCH 1/2] Fixes method signature for Utils Previously two disperate objects could be passed into utils, a `Gateway` or a `provider`. This changes the signature to always expect a `gateway` It reduces the potential impact of this change from causing a double lookup by lazy loading utils. There are most likely additional refactors that are required here but this allows the utils class not swallow unrelated errors. --- app/models/spree/gateway/braintree_vzero_base.rb | 6 +++--- .../spree/gateway/braintree_vzero_base/address.rb | 15 ++++++++++----- .../braintree_vzero_base/braintree_user.rb | 15 ++++++++++----- .../spree/gateway/braintree_vzero_base/utils.rb | 4 ++-- .../gateway/braintree_vzero_base/address_spec.rb | 2 +- spec/models/gateway/braintree_vzero_base_spec.rb | 2 +- 6 files changed, 27 insertions(+), 17 deletions(-) diff --git a/app/models/spree/gateway/braintree_vzero_base.rb b/app/models/spree/gateway/braintree_vzero_base.rb index 6a869c2c..be4c6b6e 100644 --- a/app/models/spree/gateway/braintree_vzero_base.rb +++ b/app/models/spree/gateway/braintree_vzero_base.rb @@ -152,13 +152,13 @@ def invalid_payment_error(data) Braintree::ErrorResult.new(:transaction, params: data, errors: { transaction: errors }, message: message) end - def braintree_user(provider, user, order) - Gateway::BraintreeVzeroBase::BraintreeUser.new(provider, user, order).user + def braintree_user(user, order) + Gateway::BraintreeVzeroBase::BraintreeUser.new(self, user, order).user end def token_params(provider, user, order) token_params = {} - token_params[:customer_id] = user.id if braintree_user(provider, user, order) + token_params[:customer_id] = user.id if braintree_user(user, order) currency = order.try(:currency) merchant_account_id = currency ? get_merchant_account(currency) : nil token_params[:merchant_account_id] = merchant_account_id if merchant_account_id diff --git a/app/models/spree/gateway/braintree_vzero_base/address.rb b/app/models/spree/gateway/braintree_vzero_base/address.rb index 70441500..051d33ed 100644 --- a/app/models/spree/gateway/braintree_vzero_base/address.rb +++ b/app/models/spree/gateway/braintree_vzero_base/address.rb @@ -2,16 +2,21 @@ module Spree class Gateway class BraintreeVzeroBase class Address - attr_reader :user, :utils, :request + attr_reader :user, :request - def initialize(provider, order) - @utils = Utils.new(provider, order) + def initialize(gateway, order) + @gateway = gateway + @order = order @user = order.user - @request = provider::Address + @request = gateway.provider::Address + end + + def utils + @_utils ||= Utils.new(@gateway, @order) end def create - @request.create(@utils.address_data('billing', utils.order).merge!(customer_id: user.id.to_s)) + @request.create(utils.address_data('billing', utils.order).merge!(customer_id: user.id.to_s)) end def find(braintree_address) diff --git a/app/models/spree/gateway/braintree_vzero_base/braintree_user.rb b/app/models/spree/gateway/braintree_vzero_base/braintree_user.rb index e53060a7..cbc06bc6 100644 --- a/app/models/spree/gateway/braintree_vzero_base/braintree_user.rb +++ b/app/models/spree/gateway/braintree_vzero_base/braintree_user.rb @@ -2,22 +2,27 @@ module Spree class Gateway class BraintreeVzeroBase class BraintreeUser - attr_reader :user, :spree_user, :request, :utils + attr_reader :user, :spree_user, :request delegate :shipping_address, :billing_address, to: :spree_user - def initialize(provider, spree_user, order) - @utils = Utils.new(provider, order) + def initialize(gateway, spree_user, order) @spree_user = spree_user - @request = provider::Customer + @order = order + @gateway = gateway + @request = gateway.provider::Customer begin @user = @request.find(spree_user.try(:id)) rescue end end + def utils + @_utils ||= Utils.new(@gateway, @order) + end + def register_user - @request.create(@utils.customer_data(spree_user)) + @request.create(utils.customer_data(spree_user)) end end end diff --git a/app/models/spree/gateway/braintree_vzero_base/utils.rb b/app/models/spree/gateway/braintree_vzero_base/utils.rb index 82ddfdf8..c864252e 100644 --- a/app/models/spree/gateway/braintree_vzero_base/utils.rb +++ b/app/models/spree/gateway/braintree_vzero_base/utils.rb @@ -8,14 +8,14 @@ def initialize(gateway, order) @order = order begin @customer = gateway.provider::Customer.find(order.user.id) if order.user - rescue + rescue Braintree::NotFoundError end @gateway = gateway end def get_address(address_type) if order.user && (address = order.user.send("#{address_type}_address")) - braintree_address = BraintreeVzeroBase::Address.new(gateway.provider, order) + braintree_address = BraintreeVzeroBase::Address.new(gateway, order) vaulted_duplicate = Spree::Address.vaulted_duplicates(address).first if vaulted_duplicate && braintree_address.find(vaulted_duplicate.braintree_id) diff --git a/spec/models/gateway/braintree_vzero_base/address_spec.rb b/spec/models/gateway/braintree_vzero_base/address_spec.rb index fd7aa080..aa27c3d6 100644 --- a/spec/models/gateway/braintree_vzero_base/address_spec.rb +++ b/spec/models/gateway/braintree_vzero_base/address_spec.rb @@ -4,7 +4,7 @@ let(:gateway) { create(:vzero_gateway, auto_capture: true) } let(:user) { create(:user) } let(:order) { create(:order) } - let(:braintree_address) { Spree::Gateway::BraintreeVzeroBase::Address.new(gateway.provider, order) } + let(:braintree_address) { Spree::Gateway::BraintreeVzeroBase::Address.new(gateway, order) } context '#create' do it 'creates Braintree Address' do diff --git a/spec/models/gateway/braintree_vzero_base_spec.rb b/spec/models/gateway/braintree_vzero_base_spec.rb index 11348471..ae3b328f 100644 --- a/spec/models/gateway/braintree_vzero_base_spec.rb +++ b/spec/models/gateway/braintree_vzero_base_spec.rb @@ -22,7 +22,7 @@ it 'generates token for User registered in Braintree' do user = create(:user, billing_address: create(:address)) - Spree::Gateway::BraintreeVzeroBase::BraintreeUser.new(gateway.provider, user, order).register_user + Spree::Gateway::BraintreeVzeroBase::BraintreeUser.new(gateway, user, order).register_user expect(gateway.client_token(order, user)).to_not be_nil end From 6a4227467b911037b21f6e27ebe1395bc745e030 Mon Sep 17 00:00:00 2001 From: Russell Osborne Date: Tue, 30 Jan 2018 16:28:18 -0500 Subject: [PATCH 2/2] Refactors utils customer to lazy load --- .../gateway/braintree_vzero_base/utils.rb | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/models/spree/gateway/braintree_vzero_base/utils.rb b/app/models/spree/gateway/braintree_vzero_base/utils.rb index c864252e..0b702f02 100644 --- a/app/models/spree/gateway/braintree_vzero_base/utils.rb +++ b/app/models/spree/gateway/braintree_vzero_base/utils.rb @@ -2,17 +2,19 @@ module Spree class Gateway class BraintreeVzeroBase class Utils - attr_reader :order, :customer, :gateway + attr_reader :order, :gateway def initialize(gateway, order) @order = order - begin - @customer = gateway.provider::Customer.find(order.user.id) if order.user - rescue Braintree::NotFoundError - end @gateway = gateway end + def customer + @customer ||= gateway.provider::Customer.find(order.user.id) if order.user + rescue Braintree::NotFoundError + nil + end + def get_address(address_type) if order.user && (address = order.user.send("#{address_type}_address")) braintree_address = BraintreeVzeroBase::Address.new(gateway, order) @@ -29,8 +31,8 @@ def get_address(address_type) end def get_customer - if @customer - { customer_id: @customer.id } + if customer + { customer_id: customer.id } else { customer: (payment_in_vault[:store_shipping_address_in_vault] && order.user) ? customer_data(order.user) : {} } end @@ -68,7 +70,7 @@ def customer_data(user) end def customer_payment_methods(payment_method_type) - payment_methods = @customer.try(:payment_methods) || [] + payment_methods = customer.try(:payment_methods) || [] if payment_method_type.eql?('custom') payment_methods.select { |pm| pm.is_a?(Braintree::CreditCard) }