Skip to content

Commit

Permalink
Skip setting default state when not included in available countries
Browse files Browse the repository at this point in the history
Closes #1205. In the case that default country is not included within 
available
countries, this commit leaves the states select empty, where previously 
was
filled in with default country states which didn't match with any 
available
country. The state select is later filled in by Select2, which knows 
which
country option is selected. Server side we can't assume that the first 
country
option is selected, because it doesn't play well with client side 
features, e.g.
autocomplete.
  • Loading branch information
mdesantis committed Jan 9, 2019
1 parent b1e1bbf commit 8814b86
Show file tree
Hide file tree
Showing 7 changed files with 270 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ def show
end

def edit
country_id = Spree::Country.default.id
country_id = default_country_id

@order.build_bill_address(country_id: country_id) if @order.bill_address.nil?
@order.build_ship_address(country_id: country_id) if @order.ship_address.nil?

Expand Down Expand Up @@ -43,6 +44,12 @@ def update

private

def default_country_id
country_id = Spree::Country.default.id

country_id if Spree::Country.available.pluck(:id).include?(country_id)
end

def order_params
params.require(:order).permit(
:email,
Expand Down
13 changes: 10 additions & 3 deletions backend/app/views/spree/admin/shared/_address_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,18 @@
<%= f.label :state_id, Spree::State.model_name.human %>
<span id="<%= s_or_b %>state">
<%= f.hidden_field :state_name, value: nil %>
<% states = f.object.country.try(:states).nil? ? [] : f.object.country.states %>
<%= f.text_field :state_name,
style: "display: #{f.object.country.states.empty? ? 'block' : 'none' };",
disabled: !f.object.country.states.empty?, class: 'fullwidth state_name js-state_name' %>
style: "display: #{states.empty? ? 'block' : 'none' };",
disabled: !states.empty?, class: 'fullwidth state_name js-state_name' %>
<%= f.hidden_field :state_id, value: nil %>
<%= f.collection_select :state_id, f.object.country.states.sort, :id, :name, {include_blank: true}, {class: 'custom-select fullwidth js-state_id', style: "display: #{f.object.country.states.empty? ? 'none' : 'block' };", disabled: f.object.country.states.empty?} %>
<%= f.collection_select :state_id,
states.sort,
:id, :name,
{ include_blank: true },
{ class: 'custom-select fullwidth js-state_id',
style: "display: #{states.empty? ? 'none' : 'block' };",
disabled: states.empty? } %>
</span>
</div>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,59 @@
context "with authorization" do
stub_authorization!

let(:order) { create(:order, number: "R123456789") }
context '#edit' do
context 'when order has no shipping nor billing address' do
let(:order) { create(:order, number: "R123456789", ship_address: nil, bill_address: nil) }

before { allow(Spree::Order).to receive_message_chain(:includes, :find_by!) { order } }
context "with a checkout_zone set as the country of Canada" do
let!(:united_states) { create(:country, iso: 'US', states_required: true) }
let!(:canada) { create(:country, iso: 'CA', states_required: true) }
let!(:checkout_zone) { create(:zone, name: "Checkout Zone", countries: [canada]) }

before do
Spree::Config.checkout_zone = checkout_zone.name
end

context "and default_country_iso of the Canada" do
before do
Spree::Config.default_country_iso = Spree::Country.find_by!(iso: "CA").iso
end

it 'defaults the shipping address country to Canada' do
get :edit, params: { order_id: order.number }
expect(assigns(:order).shipping_address.country_id).to eq canada.id
end

it 'defaults the billing address country to Canada' do
get :edit, params: { order_id: order.number }
expect(assigns(:order).billing_address.country_id).to eq canada.id
end
end

context "and default_country_iso of the United States" do
before do
Spree::Config.default_country_iso = Spree::Country.find_by!(iso: "US").iso
end

it 'defaults the shipping address country to nil' do
get :edit, params: { order_id: order.number }
expect(assigns(:order).shipping_address.country_id).to be_nil
end

it 'defaults the billing address country to nil' do
get :edit, params: { order_id: order.number }
expect(assigns(:order).billing_address.country_id).to be_nil
end
end
end
end
end

context "#update" do
let(:order) { create(:order, number: "R123456789") }

before { allow(Spree::Order).to receive_message_chain(:includes, :find_by!) { order } }

it "updates + progresses the order" do
expect(order).to receive(:update_attributes) { true }
expect(order).to receive(:next) { false }
Expand Down
117 changes: 117 additions & 0 deletions backend/spec/features/admin/orders/new_order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,123 @@
end
end

context 'with a checkout_zone set as the country of Canada' do
let!(:canada) { create(:country, iso: 'CA', states_required: true) }
let!(:canada_state) { create(:state, country: canada) }
let!(:checkout_zone) { create(:zone, name: 'Checkout Zone', countries: [canada]) }

before do
Spree::Country.update_all(states_required: true)
Spree::Config.checkout_zone = checkout_zone.name
end

context 'and default_country_iso of the United States' do
before do
Spree::Config.default_country_iso = Spree::Country.find_by!(iso: 'US').iso
end

it 'the shipping address country select includes only options for Canada' do
visit spree.new_admin_order_path
click_link 'Customer'
within '#shipping' do
expect(page).to have_select(
'Country',
options: ['Canada']
)
end
end

it 'does not show any shipping address state' do
visit spree.new_admin_order_path
click_link 'Customer'
within '#shipping' do
expect(page).to have_select(
'State',
disabled: true,
visible: false,
options: ['']
)
end
end

it 'the billing address country select includes only options for Canada' do
visit spree.new_admin_order_path
click_link 'Customer'
within '#billing' do
expect(page).to have_select(
'Country',
options: ['Canada']
)
end
end

it 'does not show any billing address state' do
visit spree.new_admin_order_path
click_link 'Customer'
within '#billing' do
expect(page).to have_select(
'State',
disabled: true,
visible: false,
options: ['']
)
end
end
end

context 'and default_country_iso of Canada' do
before do
Spree::Config.default_country_iso = Spree::Country.find_by!(iso: 'CA').iso
end

it 'defaults the shipping address country to Canada' do
visit spree.new_admin_order_path
click_link 'Customer'
within '#shipping' do
expect(page).to have_select(
'Country',
selected: 'Canada',
options: ['Canada']
)
end
end

it 'shows relevant shipping address states' do
visit spree.new_admin_order_path
click_link 'Customer'
within '#shipping' do
expect(page).to have_select(
'State',
options: [''] + canada.states.map(&:name)
)
end
end

it 'defaults the billing address country to Canada' do
visit spree.new_admin_order_path
click_link 'Customer'
within '#billing' do
expect(page).to have_select(
'Country',
selected: 'Canada',
options: ['Canada']
)
end
end

it 'shows relevant billing address states' do
visit spree.new_admin_order_path
click_link 'Customer'
within '#billing' do
expect(page).to have_select(
'State',
options: [''] + canada.states.map(&:name)
)
end
end
end
end

def fill_in_address
fill_in "First Name", with: "John 99"
fill_in "Last Name", with: "Doe"
Expand Down
8 changes: 1 addition & 7 deletions core/app/helpers/spree/base_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,7 @@ def taxons_tree(root_taxon, current_taxon, max_level = 1)
end

def available_countries(restrict_to_zone: Spree::Config[:checkout_zone])
checkout_zone = Zone.find_by(name: restrict_to_zone)

if checkout_zone && checkout_zone.kind == 'country'
countries = checkout_zone.country_list
else
countries = Country.all
end
countries = Spree::Country.available(restrict_to_zone: restrict_to_zone)

country_names = Carmen::Country.all.map do |country|
[country.code, country.name]
Expand Down
8 changes: 8 additions & 0 deletions core/app/models/spree/country.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ def self.default
end
end

def self.available(restrict_to_zone: Spree::Config[:checkout_zone])
checkout_zone = Zone.find_by(name: restrict_to_zone)

return checkout_zone.country_list if checkout_zone.try(:kind) == 'country'

all
end

def <=>(other)
name <=> other.name
end
Expand Down
76 changes: 76 additions & 0 deletions core/spec/models/spree/country_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,82 @@
end
end

describe '.available' do
let!(:united_states) { create(:country, iso: 'US') }
let!(:canada) { create(:country, iso: 'CA') }
let!(:italy) { create(:country, iso: 'IT') }
let!(:custom_zone) { create(:zone, name: 'Custom Zone', countries: [united_states, italy]) }

context 'with a checkout zone defined' do
context 'when checkout zone is of type country' do
let!(:checkout_zone) { create(:zone, name: 'Checkout Zone', countries: [united_states, canada]) }

before do
Spree::Config.checkout_zone = checkout_zone.name
end

context 'with no arguments' do
it 'returns "Checkout Zone" countries' do
expect(described_class.available).to contain_exactly(united_states, canada)
end
end

context 'setting nil as restricting zone' do
it 'returns all countries' do
expect(described_class.available(restrict_to_zone: nil)).to contain_exactly(united_states, canada, italy)
end
end

context 'setting "Custom Zone" as restricting zone' do
it 'returns "Custom Zone" countries' do
expect(described_class.available(restrict_to_zone: 'Custom Zone')).to contain_exactly(united_states, italy)
end
end

context 'setting "Checkout Zone" as restricting zone' do
it 'returns "Checkout Zone" countries' do
expect(described_class.available(restrict_to_zone: 'Checkout Zone')).to contain_exactly(united_states, canada)
end
end
end

context 'when checkout zone is of type state' do
let!(:state) { create(:state, country: united_states) }
let!(:checkout_zone) { create(:zone, name: 'Checkout Zone', states: [state]) }

before do
Spree::Config[:checkout_zone] = checkout_zone.name
end

context 'with no arguments' do
it 'returns all countries' do
expect(described_class.available(restrict_to_zone: nil)).to contain_exactly(united_states, canada, italy)
end
end
end
end

context 'with no checkout zone defined' do
context 'with no arguments' do
it 'returns all countries' do
expect(described_class.available).to contain_exactly(united_states, canada, italy)
end
end

context 'setting nil as restricting zone' do
it 'returns all countries' do
expect(described_class.available(restrict_to_zone: nil)).to contain_exactly(united_states, canada, italy)
end
end

context 'setting "Custom Zone" as restricting zone' do
it 'returns "Custom Zone" countries' do
expect(described_class.available(restrict_to_zone: 'Custom Zone')).to contain_exactly(united_states, italy)
end
end
end
end

describe '#prices' do
let(:country) { create(:country) }
subject { country.prices }
Expand Down

0 comments on commit 8814b86

Please sign in to comment.