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

Update to Solidus 3.0 #286

Merged
merged 13 commits into from
Apr 26, 2021
5 changes: 4 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ require:

AllCops:
NewCops: enable
TargetRubyVersion: 2.4
TargetRubyVersion: 2.5

Layout/FirstArgumentIndentation:
EnforcedStyle: consistent
Expand All @@ -17,6 +17,9 @@ Layout/FirstHashElementIndentation:
Layout/MultilineMethodCallIndentation:
EnforcedStyle: indented

Naming/VariableNumber:
Enabled: false

# We use this extensively, the alternatives are not viable or desirable.
RSpec/AnyInstance:
Enabled: false
Expand Down
4 changes: 1 addition & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ gem 'solidus_auth_devise'
gem 'mini_racer'
gem 'sassc-rails', platforms: :mri

# bourbon 5 doesn't work under sassc
# https://github.com/thoughtbot/bourbon/issues/1047
gem 'bourbon', '<5'
gem 'bourbon'

case ENV['DB']
when 'mysql'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
module SolidusPaypalBraintree
module BraintreeCheckoutHelper
def braintree_3ds_options_for(order)
ship_address = order.ship_address
bill_address = order.bill_address

ship_address = SolidusPaypalBraintree::Address.new(order.ship_address)
bill_address = SolidusPaypalBraintree::Address.new(order.bill_address)
{
nonce: nil, # populated after tokenization
bin: nil, # populated after tokenization
Expand All @@ -19,7 +18,7 @@ def braintree_3ds_options_for(order)
streetAddress: bill_address.address1,
extendedAddress: bill_address.address2,
locality: bill_address.city,
region: bill_address.state&.name,
region: bill_address.state&.abbr,
postalCode: bill_address.zipcode,
countryCodeAlpha2: bill_address.country&.iso,
},
Expand All @@ -31,7 +30,7 @@ def braintree_3ds_options_for(order)
streedAddress: ship_address.address1,
extendedAddress: ship_address.address2,
locality: ship_address.city,
region: ship_address.state&.name,
region: ship_address.state&.abbr,
postalCode: ship_address.zipcode,
countryCodeAlpha2: ship_address.country&.iso,
}
Expand Down
52 changes: 43 additions & 9 deletions app/models/solidus_paypal_braintree/address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,61 @@

module SolidusPaypalBraintree
class Address
delegate :address1,
:address2,
:city,
:country,
:phone,
:state,
:zipcode,
to: :spree_address

def self.split_name(name)
if defined?(Spree::Address::Name)
address_name = Spree::Address::Name.new(name)
[address_name.first_name, address_name.last_name]
else
name.strip.split(' ', 2)
end
end

def initialize(spree_address)
@spree_address = spree_address
end

def to_json(*_args)
address_hash = {
line1: spree_address.address1,
line2: spree_address.address2,
city: spree_address.city,
postalCode: spree_address.zipcode,
countryCode: spree_address.country.iso,
phone: spree_address.phone,
recipientName: spree_address.full_name
line1: address1,
line2: address2,
city: city,
postalCode: zipcode,
countryCode: country.iso,
phone: phone,
recipientName: "#{firstname} #{lastname}"
}

if ::Spree::Config.address_requires_state && spree_address.country.states_required
address_hash[:state] = spree_address.state.name
if ::Spree::Config.address_requires_state && country.states_required
address_hash[:state] = state.name
end
address_hash.to_json
end

def firstname
if SolidusSupport.combined_first_and_last_name_in_address?
self.class.split_name(spree_address.name).first
else
spree_address.firstname
end
end

def lastname
if SolidusSupport.combined_first_and_last_name_in_address?
self.class.split_name(spree_address.name).last
else
spree_address.lastname
end
end

private

attr_reader :spree_address
Expand Down
4 changes: 4 additions & 0 deletions app/models/solidus_paypal_braintree/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ class Configuration < ::Spree::Base
messaging: { availables: %w[true false], default: 'false' }
}.freeze

unless respond_to?(:preference)
include ::Spree::Preferences::Persistable
end

belongs_to :store, class_name: 'Spree::Store'

validates :store, presence: true
Expand Down
30 changes: 17 additions & 13 deletions app/models/solidus_paypal_braintree/transaction_address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,24 @@ 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
if SolidusSupport.combined_first_and_last_name_in_address?
address.name = begin
if first_name.nil?
Copy link
Member

Choose a reason for hiding this comment

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

Why this check? In Solidus 2.11 we'll have both first_name and name always synced and this is probably always false. In Solidus 3, there's no reason to check first_name, which is no more used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here first_name is an instance attribute of SolidusPaypalBraintree::TransactionAddress (together with last_name and name). If I recall correctly these values are returned by Braintree itself and maybe they can be valorized differently according to the API version (that's why we need the check on its presence).

Copy link
Member

Choose a reason for hiding this comment

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

Ah makes sense!

name
else
[first_name, last_name].join(" ")
end
end
else
first_name, last_name = split_name(name)
address.first_name = first_name
address.last_name = last_name || "(left blank)"
::Spree::Deprecation.warn("first_name and last_name are deprecated. Use name instead.", caller)
if first_name.nil?
first, last = SolidusPaypalBraintree::Address.split_name(name)
address.firstname = first
address.lastname = last || "(left blank)"
else
address.firstname = first_name
address.lastname = last_name || "(left blank)"
end
end

if spree_state
Expand All @@ -71,10 +79,6 @@ 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
11 changes: 10 additions & 1 deletion lib/solidus_paypal_braintree/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,17 @@
# As we match the body in our VCR settings VCR can not match the request anymore and therefore cannot replay existing
# cassettes.
#

factory :address do
zipcode { '21088-0255' }
lastname { 'Doe' }

if SolidusSupport.combined_first_and_last_name_in_address?
transient do
firstname { "John" }
lastname { "Doe" }
end

name { "#{firstname} #{lastname}" }
end
end
end
8 changes: 4 additions & 4 deletions solidus_paypal_braintree.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Gem::Specification.new do |s|
s.email = 'braintree+gemfile@stembolt.com'
s.homepage = 'https://github.com/solidusio/solidus_paypal_braintree'

s.required_ruby_version = '~> 2.4'
s.required_ruby_version = '>= 2.5'

s.files = Dir.chdir(File.expand_path(__dir__)) do
`git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) }
Expand All @@ -31,9 +31,9 @@ Gem::Specification.new do |s|

s.add_dependency 'activemerchant', '~> 1.48'
s.add_dependency 'braintree', '~> 2.65'
s.add_dependency 'solidus_api', ['>= 2.0.0', '< 3']
s.add_dependency 'solidus_core', ['>= 2.0.0', '< 3']
s.add_dependency 'solidus_support', '~> 0.5.0'
s.add_dependency 'solidus_api', ['>= 2.0.0', '< 4']
s.add_dependency 'solidus_core', ['>= 2.0.0', '< 4']
s.add_dependency 'solidus_support', ['>= 0.8.1', '< 1']

s.add_development_dependency 'selenium-webdriver'
s.add_development_dependency 'solidus_dev_support'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
# the transaction import only creates 1 new one
order
expect { post_create }.to change { Spree::Address.count }.by(1)
expect(Spree::Address.last.full_name).to eq "Wade Wilson"
expect(Spree::Address.last.address1).to eq "123 Fake Street"
end
end

Expand Down
20 changes: 20 additions & 0 deletions spec/models/solidus_paypal_braintree/address_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
require 'spec_helper'

RSpec.describe SolidusPaypalBraintree::Address do
describe "::split_name" do
subject { described_class.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

describe '#to_json' do
subject(:address_json) { JSON.parse(described_class.new(spree_address).to_json) }

Expand Down
2 changes: 1 addition & 1 deletion spec/models/solidus_paypal_braintree/gateway_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
let(:user) { create :user }

let(:source) do
SolidusPaypalBraintree::Source.new(
SolidusPaypalBraintree::Source.create!(
nonce: 'fake-valid-nonce',
user: user,
payment_type: payment_type,
Expand Down
34 changes: 8 additions & 26 deletions spec/models/solidus_paypal_braintree/transaction_address_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,34 +219,16 @@
end
end

context 'when using first_name and last_name' do
let(:address_params) { super().merge({ first_name: "Bruce", last_name: "Wayne" }) }
unless SolidusSupport.combined_first_and_last_name_in_address?
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)
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"]
subject
end
end
end
end
Expand Down
3 changes: 3 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

require File.expand_path('dummy/config/environment.rb', __dir__)

require 'rails-controller-testing'
Rails::Controller::Testing.install

# Requires factories and other useful helpers defined in spree_core.
require 'solidus_dev_support/rspec/feature_helper'

Expand Down