Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Postal Code Format Validation (and Twitter CLDR dependency) #2233

Merged
merged 3 commits into from
Sep 28, 2017
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
12 changes: 1 addition & 11 deletions core/app/models/spree/address.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
require 'twitter_cldr'

module Spree
# `Spree::Address` provides the foundational ActiveRecord model for recording and
# validating address information for `Spree::Order`, `Spree::Shipment`,
Expand All @@ -15,7 +13,7 @@ class Address < Spree::Base
validates :zipcode, presence: true, if: :require_zipcode?
validates :phone, presence: true, if: :require_phone?

validate :state_validate, :postal_code_validate
validate :state_validate

alias_attribute :first_name, :firstname
alias_attribute :last_name, :lastname
Expand Down Expand Up @@ -211,13 +209,5 @@ def state_validate
# ensure at least one state field is populated
errors.add :state, :blank if state.blank? && state_name.blank?
end

def postal_code_validate
return if country.blank? || country.iso.blank? || !require_zipcode?
return if !TwitterCldr::Shared::PostalCodes.territories.include?(country.iso.downcase.to_sym)

postal_code = TwitterCldr::Shared::PostalCodes.for_territory(country.iso)
errors.add(:zipcode, :invalid) if !postal_code.valid?(zipcode.to_s)
end
end
end
3 changes: 1 addition & 2 deletions core/lib/spree/testing_support/factories/address_factory.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
require 'spree/testing_support/factories/state_factory'
require 'spree/testing_support/factories/country_factory'
require 'twitter_cldr'

FactoryGirl.define do
factory :address, class: Spree::Address do
Expand All @@ -16,7 +15,7 @@
address1 '10 Lovely Street'
address2 'Northwest'
city 'Herndon'
zipcode { TwitterCldr::Shared::PostalCodes.for_territory(country_iso_code).sample.first }
zipcode { FFaker::AddressUS.zip_code }
phone '555-555-0199'
alternative_phone '555-555-0199'

Expand Down
1 change: 0 additions & 1 deletion core/solidus_core.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ Gem::Specification.new do |s|
s.add_dependency 'ransack', '~> 1.8'
s.add_dependency 'state_machines-activerecord', '~> 0.4'
s.add_dependency 'stringex', '~> 1.5.1'
s.add_dependency 'twitter_cldr', '>= 3.0', '< 5'

s.add_development_dependency 'email_spec', '~> 1.6'
end
35 changes: 0 additions & 35 deletions core/spec/models/spree/address_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,41 +103,6 @@
expect(address.errors['zipcode']).to include("can't be blank")
end

context "zipcode validation" do
it "validates the zipcode" do
allow(address.country).to receive(:iso).and_return('US')
address.zipcode = 'abc'
address.valid?
expect(address.errors['zipcode']).to include('is invalid')
end

context 'does not validate' do
it 'does not have a country' do
address.country = nil
address.valid?
expect(address.errors['zipcode']).not_to include('is invalid')
end

it 'does not have an iso' do
allow(address.country).to receive(:iso).and_return(nil)
address.valid?
expect(address.errors['zipcode']).not_to include('is invalid')
end

it 'does not have a zipcode' do
address.zipcode = ""
address.valid?
expect(address.errors['zipcode']).not_to include('is invalid')
end

it 'does not have a supported country iso' do
allow(address.country).to receive(:iso).and_return('BO')
address.valid?
expect(address.errors['zipcode']).not_to include('is invalid')
end
end
end

context "phone not required" do
before { allow(address).to receive_messages require_phone?: false }

Expand Down