From 522df35e6d73fe4f4f6569891a7ab2f77bc0419c Mon Sep 17 00:00:00 2001 From: Sean Date: Thu, 22 Oct 2020 14:19:37 -0500 Subject: [PATCH] Update transaction_address to support name attribute Solidus is moving away from using first_name and last_name, in favor of just "name". This updates the paypal JS to send a name to transaction_addess, rather than splitting it on the spot. transaction_address is responsible for either: * Splitting the name, if the user is on Solidus 2.10 or below -or- * Passing the name directly to the address, if the user is on Solidus 2.11 or higher - where the name parameter is supported. Also deprecates using first_name and last_name in transaction_address, in case someone has overridden `paypal_button` --- .../solidus_paypal_braintree/paypal_button.js | 15 ++--- .../transaction_address.rb | 26 ++++++-- .../transactions_controller.rb | 4 +- .../transactions_controller_spec.rb | 3 +- .../transaction_address_spec.rb | 59 +++++++++++++++---- .../transaction_import_spec.rb | 9 +-- .../transaction_spec.rb | 3 +- 7 files changed, 80 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/solidus_paypal_braintree/paypal_button.js b/app/assets/javascripts/solidus_paypal_braintree/paypal_button.js index 4b86d26c..fa1e5e79 100644 --- a/app/assets/javascripts/solidus_paypal_braintree/paypal_button.js +++ b/app/assets/javascripts/solidus_paypal_braintree/paypal_button.js @@ -125,23 +125,18 @@ SolidusPaypalBraintree.PaypalButton.prototype._transactionParams = function(payl * @param {object} payload - The payload returned by Braintree after tokenization */ SolidusPaypalBraintree.PaypalButton.prototype._addressParams = function(payload) { - var first_name, last_name; + var name; var payload_address = payload.details.shippingAddress || payload.details.billingAddress; if (!payload_address) return {}; if (payload_address.recipientName) { - first_name = payload_address.recipientName.split(" ")[0]; - last_name = payload_address.recipientName.split(" ")[1]; - } - - if (!first_name || !last_name) { - first_name = payload.details.firstName; - last_name = payload.details.lastName; + name = payload_address.recipientName + } else { + name = payload.details.firstName + " " + payload.details.lastName; } return { - "first_name" : first_name, - "last_name" : last_name, + "name" : name, "address_line_1" : payload_address.line1, "address_line_2" : payload_address.line2, "city" : payload_address.city, diff --git a/app/models/solidus_paypal_braintree/transaction_address.rb b/app/models/solidus_paypal_braintree/transaction_address.rb index e06c05e5..515ae28a 100644 --- a/app/models/solidus_paypal_braintree/transaction_address.rb +++ b/app/models/solidus_paypal_braintree/transaction_address.rb @@ -8,11 +8,11 @@ class TransactionAddress include ActiveModel::Validations::Callbacks include SolidusPaypalBraintree::CountryMapper - attr_accessor :country_code, :last_name, :first_name, - :city, :zip, :state_code, :address_line_1, :address_line_2 + attr_accessor :country_code, :name, :city, :zip, :state_code, + :address_line_1, :address_line_2, :first_name, :last_name - validates :first_name, :last_name, :address_line_1, :city, :zip, - :country_code, presence: true + validates :address_line_1, :city, :zip, :country_code, presence: true + validates :name, presence: true, unless: ->(address){ address.first_name.present? } before_validation do self.country_code = country_code.presence || "us" @@ -44,8 +44,6 @@ def spree_state def to_spree_address address = ::Spree::Address.new( - first_name: first_name, - last_name: last_name, city: city, country: spree_country, address1: address_line_1, @@ -53,6 +51,18 @@ def to_spree_address zipcode: zip ) + if !first_name.nil? + ::Spree::Deprecation.warn("first_name and last_name are deprecated. Use name instead.", caller) + address.first_name = first_name + address.last_name = last_name || "(left blank)" + elsif address.respond_to? :name + address.name = name + else + first_name, last_name = split_name(name) + address.first_name = first_name + address.last_name = last_name || "(left blank)" + end + if spree_state address.state = spree_state else @@ -61,6 +71,10 @@ def to_spree_address address end + def split_name(name) + name.strip.split(' ', 2) + end + # Check to see if this address should match to a state model in the database def should_match_state_model? spree_country&.states_required? diff --git a/lib/controllers/frontend/solidus_paypal_braintree/transactions_controller.rb b/lib/controllers/frontend/solidus_paypal_braintree/transactions_controller.rb index 65751e72..c317a033 100644 --- a/lib/controllers/frontend/solidus_paypal_braintree/transactions_controller.rb +++ b/lib/controllers/frontend/solidus_paypal_braintree/transactions_controller.rb @@ -10,8 +10,8 @@ class InvalidImportError < StandardError; end :phone, :email, { address_attributes: [ - :country_code, :country_name, :last_name, :first_name, - :city, :zip, :state_code, :address_line_1, :address_line_2 + :country_code, :country_name, :name, :city, :zip, :state_code, + :address_line_1, :address_line_2, :first_name, :last_name ] } ].freeze diff --git a/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb b/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb index f3da993d..50461933 100644 --- a/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb +++ b/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb @@ -41,8 +41,7 @@ phone: "1112223333", email: "batman@example.com", address_attributes: { - first_name: "Wade", - last_name: "Wilson", + name: "Wade Wilson", address_line_1: "123 Fake Street", city: "Seattle", zip: "98101", diff --git a/spec/models/solidus_paypal_braintree/transaction_address_spec.rb b/spec/models/solidus_paypal_braintree/transaction_address_spec.rb index 085cd359..ef26c0f4 100644 --- a/spec/models/solidus_paypal_braintree/transaction_address_spec.rb +++ b/spec/models/solidus_paypal_braintree/transaction_address_spec.rb @@ -8,8 +8,7 @@ let(:valid_attributes) do { - first_name: "Bruce", - last_name: "Wayne", + name: "Bruce Wayne", address_line_1: "42 Spruce Lane", city: "Gotham", zip: "98201", @@ -32,14 +31,8 @@ it { is_expected.to be false } end - context "without first_name" do - let(:valid_attributes) { super().except(:first_name) } - - it { is_expected.to be false } - end - - context "without last_name" do - let(:valid_attributes) { super().except(:last_name) } + context "without name" do + let(:valid_attributes) { super().except(:name) } it { is_expected.to be false } end @@ -84,6 +77,12 @@ expect(address.country_code).to eq "us" end end + + context "with a one word name" do + let(:valid_attributes) { super().merge({ name: "Bruce" }) } + + it { is_expected.to be true } + end end describe "#attributes=" do @@ -190,8 +189,15 @@ end describe '#to_spree_address' do - subject { described_class.new(country_code: 'US', state_code: 'NY').to_spree_address } + subject { described_class.new(address_params).to_spree_address } + let(:address_params) do + { + country_code: 'US', + state_code: 'NY', + name: "Alfred" + } + end let!(:us) { create :country, iso: 'US' } it { is_expected.to be_a Spree::Address } @@ -212,5 +218,36 @@ expect(subject.state_text).to eq 'NY' end end + + context 'when using first_name and last_name' do + let(:address_params) { super().merge({ first_name: "Bruce", last_name: "Wayne" }) } + + it 'displays a deprecation warning' do + expect(Spree::Deprecation).to receive(:warn). + with("first_name and last_name are deprecated. Use name instead.", any_args) + + subject + end + end + end + + describe "#split" do + subject { described_class.new.split_name(name) } + + context "with a one word name" do + let(:name) { "Bruce" } + + it "correctly splits" do + expect(subject).to eq ["Bruce"] + end + end + + context "with a multi word name" do + let(:name) { "Bruce Wayne The Batman" } + + it "correctly splits" do + expect(subject).to eq ["Bruce", "Wayne The Batman"] + end + end end end diff --git a/spec/models/solidus_paypal_braintree/transaction_import_spec.rb b/spec/models/solidus_paypal_braintree/transaction_import_spec.rb index 62098884..94b6e320 100644 --- a/spec/models/solidus_paypal_braintree/transaction_import_spec.rb +++ b/spec/models/solidus_paypal_braintree/transaction_import_spec.rb @@ -31,8 +31,7 @@ context "with invalid address" do let(:transaction_address) do SolidusPaypalBraintree::TransactionAddress.new( - first_name: "Bruce", - last_name: "Wayne", + name: "Bruce Wayne", address_line_1: "42 Spruce Lane", city: "Gotham", state_code: "WA", @@ -187,8 +186,7 @@ let(:transaction_address) do SolidusPaypalBraintree::TransactionAddress.new( country_code: 'US', - last_name: 'Venture', - first_name: 'Thaddeus', + name: 'Thaddeus Venture', city: 'New York', state_code: 'NY', address_line_1: '350 5th Ave', @@ -247,8 +245,7 @@ let(:transaction_address) do SolidusPaypalBraintree::TransactionAddress.new( country_code: 'US', - last_name: 'Venture', - first_name: 'Thaddeus', + name: 'Thaddeus Venture', city: 'New York', state_code: 'NY', address_line_1: '350 5th Ave' diff --git a/spec/models/solidus_paypal_braintree/transaction_spec.rb b/spec/models/solidus_paypal_braintree/transaction_spec.rb index 0d8a8605..801b0b1b 100644 --- a/spec/models/solidus_paypal_braintree/transaction_spec.rb +++ b/spec/models/solidus_paypal_braintree/transaction_spec.rb @@ -15,8 +15,7 @@ let(:valid_address_attributes) do { address_attributes: { - first_name: "Bruce", - last_name: "Wayne", + name: "Bruce Wayne", address_line_1: "42 Spruce Lane", city: "Gotham", zip: "98201",