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

Fix Country factory states_required attribute #4272

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
5 changes: 3 additions & 2 deletions api/spec/requests/spree/api/orders_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -523,14 +523,15 @@ module Spree::Api
let(:billing_address) {
{ name: "Tiago Motta", address1: "Av Paulista",
city: "Sao Paulo", zipcode: "01310-300", phone: "12345678",
country_id: country.id }
country_id: country.id, state_id: state.id }
}
let(:shipping_address) {
{ name: "Tiago Motta", address1: "Av Paulista",
city: "Sao Paulo", zipcode: "01310-300", phone: "12345678",
country_id: country.id }
country_id: country.id, state_id: state.id }
}
let(:country) { create(:country, { name: "Brazil", iso_name: "BRAZIL", iso: "BR", iso3: "BRA", numcode: 76 }) }
let(:state) { create(:state, country_iso: 'BR') }

before { allow_any_instance_of(Spree::Order).to receive_messages user: current_api_user }

Expand Down
12 changes: 8 additions & 4 deletions api/spec/requests/spree/api/users_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,16 @@ module Spree::Api

it "can update own details" do
country = create(:country)
state = create(:state)

put spree.api_user_path(user.id), params: { token: user.spree_api_key, user: {
email: "mine@example.com",
bill_address_attributes: {
name: 'First Last',
address1: '1 Test Rd',
city: 'City',
country_id: country.id,
state_id: 1,
state_id: state.id,
zipcode: '55555',
phone: '5555555555'
},
Expand All @@ -60,7 +62,7 @@ module Spree::Api
address1: '1 Test Rd',
city: 'City',
country_id: country.id,
state_id: 1,
state_id: state.id,
zipcode: '55555',
phone: '5555555555'
}
Expand All @@ -72,6 +74,8 @@ module Spree::Api

it "can update own details in JSON with unwrapped parameters (Rails default)" do
country = create(:country)
state = create(:state)

put spree.api_user_path(user.id),
headers: { "CONTENT_TYPE": "application/json" },
params: {
Expand All @@ -82,7 +86,7 @@ module Spree::Api
address1: '1 Test Rd',
city: 'City',
country_id: country.id,
state_id: 1,
state_id: state.id,
zipcode: '55555',
phone: '5555555555'
},
Expand All @@ -91,7 +95,7 @@ module Spree::Api
address1: '1 Test Rd',
city: 'City',
country_id: country.id,
state_id: 1,
state_id: state.id,
zipcode: '55555',
phone: '5555555555'
}
Expand Down
20 changes: 9 additions & 11 deletions core/lib/spree/testing_support/factories/address_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,19 @@
phone { '555-555-0199' }
alternative_phone { '555-555-0199' }

state do |address|
Spree::State.joins(:country).where('spree_countries.iso = (?)', country_iso_code).find_by(abbr: state_code) ||
address.association(
:state,
strategy: :create,
country_iso: country_iso_code,
state_code: state_code
)
end

country do |address|
if address.state
address.state.country
else
address.association(:country, strategy: :create, iso: country_iso_code)
Spree::Country.find_by(iso: country_iso_code) ||
address.association(:country, strategy: :create, iso: country_iso_code)
end
end

after(:build) do |address, evaluator|
if address&.country&.states_required? && address.state.nil? && address.state_name.nil?
address.state = address.country.states.find_by(abbr: evaluator.state_code) ||
create(:state, country_iso: address.country.iso, state_code: evaluator.state_code)
end
end
end
Expand Down
3 changes: 1 addition & 2 deletions core/lib/spree/testing_support/factories/country_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
iso3 { carmen_country.alpha_3_code }
numcode { carmen_country.numeric_code }

# FIXME: We should set states required, but it causes failing tests
# states_required { carmen_country.subregions? }
states_required { carmen_country.subregions? }
end
end
10 changes: 8 additions & 2 deletions core/lib/spree/testing_support/factories/state_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@
carmen_subregion do
carmen_country = Carmen::Country.coded(country.iso)

carmen_country.subregions.coded(state_code) ||
carmen_country.subregions.sort_by(&:name).first ||
unless carmen_country.subregions?
fail("Country #{country.iso} has no subregions")
end

carmen_regions = carmen_country.subregions
carmen_regions = carmen_regions.flat_map(&:subregions) if carmen_regions.first.subregions?
RyanofWoods marked this conversation as resolved.
Show resolved Hide resolved
region_collection = Carmen::RegionCollection.new(carmen_regions)

region_collection.coded(state_code) || region_collection.sort_by(&:name).first
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,65 @@
end
end

describe 'when passing in a state and country' do
subject { build(:address, country_iso_code: country_iso_code, state_code: state_code) }
context 'states and countries' do
describe 'country requiring a state' do
let(:state) { create(:state, country_iso: 'IT', state_code: 'PE' )}
let(:country) { create(:country, iso: 'IT' )}

context 'when the country has a state with proper code' do
let(:country_iso_code) { "US" }
let(:state_code) { "NY" }
context 'when given a state but no country' do
subject { build(:address, state: state) }

it 'works' do
it 'infers the country from the state' do
expect(subject).to be_valid
expect(subject.state.abbr).to eq("PE")
expect(subject.country.iso).to eq("IT")
end
end

context 'when given a country but no state' do
subject { build(:address, country: country) }

it 'automatically finds or creates an appropriate state' do
expect(subject).to be_valid
expect(subject.state.abbr).to eq("AL")
expect(subject.country.iso).to eq("IT")
end
end

context 'when given a country, no state but a state_name' do
subject { build(:address, country: country, state_name: 'Bogus state') }

it 'does not automatically find or create an appropriate state' do
expect(subject).to be_valid
expect(subject.state).to be_nil
expect(subject.state_name).to eq('Bogus state')
end
end
end

describe 'country not requiring a state' do
subject { build(:address, country: country) }
let(:country) { create(:country, iso: 'AI' )}

it 'does not automatically find or create an appropriate state' do
expect(subject).to be_valid
expect(subject.state.abbr).to eq("NY")
expect(subject.country.iso).to eq("US")
expect(subject.state).to be_nil
expect(subject.country.iso).to eq("AI")
end
end

describe 'when passing in a state and country ISO' do
subject { build(:address, country_iso_code: country_iso_code, state_code: state_code) }

context 'when the country has a state with proper code' do
let(:country_iso_code) { "US" }
let(:state_code) { "NY" }

it 'works' do
expect(subject).to be_valid
expect(subject.state.abbr).to eq("NY")
expect(subject.country.iso).to eq("US")
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,26 @@
end
end

describe 'for a country with nested carmen states' do
context 'when not given a state_iso' do
let(:state) { build(:state, country_iso: "IT") }

it 'creates the first state for that country it finds in carmen' do
expect(state.abbr).to eq("AL")
expect(state.name).to eq("Alessandria")
end
end

context 'when given a state_iso' do
let(:state) { build(:state, country_iso: "IT", state_code: 'PE' ) }

it 'finds the corresponding state' do
expect(state.abbr).to eq("PE")
expect(state.name).to eq("Pescara")
end
end
end

describe 'when given a country record' do
let(:country) { build(:country, iso: "DE") }
let(:state) { build(:state, country: country) }
Expand Down