Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Update transaction_address to support name attribute #271

Merged
merged 1 commit into from
Oct 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions app/assets/javascripts/solidus_paypal_braintree/paypal_button.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 20 additions & 6 deletions app/models/solidus_paypal_braintree/transaction_address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -44,15 +44,25 @@ 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,
address2: address_line_2,
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have something in Solidus Core that does the same thing? I recall we have a class for that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Solidus 2.11, there's a Spree::Address::Name virtual attribute which handles splitting, it's what line 58-59 handle. I'm not aware of any class that handles name splitting before 2.11 - at least, I took a quick look just now and couldn't find one!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use that class if it's defined. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

address.name creates a new instance of Spree::Address::Name and sets the first and last names accordingly: https://github.com/solidusio/solidus/blob/44da6ff1061ac8dfd339f02228bbd921b4a4c006/core/app/models/spree/address.rb#L204-L210 - so above, if that class is available, we're using it as the name splitter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you are 100% right, thanks Sean!

address.first_name = first_name
address.last_name = last_name || "(left blank)"
end

if spree_state
address.state = spree_state
else
Expand All @@ -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?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@
phone: "1112223333",
email: "[email protected]",
address_attributes: {
first_name: "Wade",
last_name: "Wilson",
name: "Wade Wilson",
address_line_1: "123 Fake Street",
city: "Seattle",
zip: "98101",
Expand Down
59 changes: 48 additions & 11 deletions spec/models/solidus_paypal_braintree/transaction_address_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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'
Expand Down
3 changes: 1 addition & 2 deletions spec/models/solidus_paypal_braintree/transaction_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down