diff --git a/api/app/views/spree/api/config/show.v1.rabl b/api/app/views/spree/api/config/show.v1.rabl index 4e28ef4c345..3e7121af74c 100644 --- a/api/app/views/spree/api/config/show.v1.rabl +++ b/api/app/views/spree/api/config/show.v1.rabl @@ -1,2 +1,3 @@ object false -node(:default_country_id) { Spree::Config[:default_country_id] } \ No newline at end of file +node(:default_country_id) { Spree::Country.default.id } +node(:default_country_iso) { Spree::Config[:default_country_iso] } diff --git a/api/spec/controllers/spree/api/config_controller_spec.rb b/api/spec/controllers/spree/api/config_controller_spec.rb index b5730dfae0d..6f0e39bae16 100644 --- a/api/spec/controllers/spree/api/config_controller_spec.rb +++ b/api/spec/controllers/spree/api/config_controller_spec.rb @@ -2,6 +2,7 @@ module Spree describe Api::ConfigController, type: :controller do + let!(:default_country) { create :country, iso: "US"} render_views before do @@ -17,7 +18,8 @@ module Spree it "returns some configuration settings" do api_get :show expect(response).to be_success - expect(json_response["default_country_id"]).to eq(Spree::Config[:default_country_id]) + expect(json_response["default_country_iso"]).to eq("US") + expect(json_response["default_country_id"]).to eq(default_country.id) end end end diff --git a/backend/app/controllers/spree/admin/stock_locations_controller.rb b/backend/app/controllers/spree/admin/stock_locations_controller.rb index 4c5d240e939..900ce49926e 100644 --- a/backend/app/controllers/spree/admin/stock_locations_controller.rb +++ b/backend/app/controllers/spree/admin/stock_locations_controller.rb @@ -6,15 +6,11 @@ class StockLocationsController < ResourceController private def set_country - if Spree::Config[:default_country_id].present? - @stock_location.country = Spree::Country.find(Spree::Config[:default_country_id]) - else - @stock_location.country = Spree::Country.find_by!(iso: 'US') - end + @stock_location.country = Spree::Country.default rescue ActiveRecord::RecordNotFound - flash[:error] = Spree.t(:stock_locations_need_a_default_country) - redirect_to(admin_stock_locations_path) && return + flash[:error] = Spree.t(:stock_locations_need_a_default_country) + redirect_to(admin_stock_locations_path) && return end end end diff --git a/backend/app/views/spree/admin/shared/_configuration_menu.html.erb b/backend/app/views/spree/admin/shared/_configuration_menu.html.erb index d575477b38c..cdf0fbbd9df 100644 --- a/backend/app/views/spree/admin/shared/_configuration_menu.html.erb +++ b/backend/app/views/spree/admin/shared/_configuration_menu.html.erb @@ -26,7 +26,7 @@ <% end %> <% if can?(:display, Spree::State) %> - <% if country = Spree::Country.find_by_id(Spree::Config[:default_country_id]) || Spree::Country.first %> + <% if country = Spree::Country.find_by(iso: Spree::Config[:default_country_iso]) || Spree::Country.first %> <%= configurations_sidebar_menu_item Spree::State.model_name.human(count: :other), admin_country_states_path(country) %> <% end %> <% end %> diff --git a/backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb b/backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb index d9fe5596291..d3469b50911 100644 --- a/backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb @@ -14,10 +14,11 @@ module Admin end end - context "with a default country present" do + context "with a default country other than the US present" do + let(:country) { create :country, iso: "BR" } + before do - country = FactoryGirl.create(:country) - Spree::Config[:default_country_id] = country.id + Spree::Config[:default_country_iso] = country.iso end it "can create a new stock location" do diff --git a/backend/spec/features/admin/orders/customer_details_spec.rb b/backend/spec/features/admin/orders/customer_details_spec.rb index b040a2e2f95..c9194e277e2 100644 --- a/backend/spec/features/admin/orders/customer_details_spec.rb +++ b/backend/spec/features/admin/orders/customer_details_spec.rb @@ -48,7 +48,7 @@ context "editing an order" do before do configure_spree_preferences do |config| - config.default_country_id = country.id + config.default_country_iso = country.iso config.company = true end @@ -111,12 +111,12 @@ end context "country associated was removed" do - let(:brazil) { create(:country, iso: "BRA", name: "Brazil") } + let(:brazil) { create(:country, iso: "BR", name: "Brazil") } before do order.bill_address.country.destroy configure_spree_preferences do |config| - config.default_country_id = brazil.id + config.default_country_iso = brazil.iso end end diff --git a/core/app/models/spree/app_configuration.rb b/core/app/models/spree/app_configuration.rb index 03ba57d9e75..fdb254b83fe 100644 --- a/core/app/models/spree/app_configuration.rb +++ b/core/app/models/spree/app_configuration.rb @@ -117,10 +117,15 @@ class AppConfiguration < Preferences::Configuration preference :currency, :string, default: "USD" # @!attribute [rw] default_country_id - # @deprecated + # @deprecated Use the default country ISO preference instead # @return [Integer,nil] id of {Country} to be selected by default in dropdowns (default: nil) preference :default_country_id, :integer + # @!attribute [rw] default_country_iso + # Default customer country ISO code + # @return [String] Two-letter ISO code of a {Spree::Country} to assumed as the country of an unidentified customer (default: "US") + preference :default_country_iso, :string, default: 'US' + # @!attribute [rw] expedited_exchanges # Kicks off an exchange shipment upon return authorization save. # charge customer if they do not return items within timely manner. diff --git a/core/app/models/spree/country.rb b/core/app/models/spree/country.rb index ba37d2a7952..2635489d797 100644 --- a/core/app/models/spree/country.rb +++ b/core/app/models/spree/country.rb @@ -13,9 +13,12 @@ def self.states_required_by_country_id end def self.default - if default_country_id = Spree::Config[:default_country_id] - find_by_id(default_country_id) - end || first + if Spree::Config.default_country_id + ActiveSupport::Deprecation.warn("Setting your default country via its ID is deprecated. Please set your default country via the `default_country_iso` setting.", caller) + find_by(id: Spree::Config.default_country_id) || find_by!(iso: Spree::Config.default_country_iso) + else + find_by!(iso: Spree::Config.default_country_iso) + end end def <=>(other) diff --git a/core/db/default/spree/countries.rb b/core/db/default/spree/countries.rb index 70dd06be305..c33b4ec850b 100644 --- a/core/db/default/spree/countries.rb +++ b/core/db/default/spree/countries.rb @@ -15,5 +15,3 @@ ActiveRecord::Base.transaction do Spree::Country.create!(countries) end - -Spree::Config[:default_country_id] ||= Spree::Country.find_by(iso: "US").id diff --git a/core/spec/models/spree/address_spec.rb b/core/spec/models/spree/address_spec.rb index 8b51f690bd8..dbd2fe7421f 100644 --- a/core/spec/models/spree/address_spec.rb +++ b/core/spec/models/spree/address_spec.rb @@ -165,18 +165,20 @@ context 'has a default country' do before do - Spree::Config[:default_country_id] = default_country.id + Spree::Config[:default_country_iso] = default_country.iso end - it "sets up a new record with Spree::Config[:default_country_id]" do + it "sets up a new record with Spree::Config[:default_country_iso]" do expect(Spree::Address.build_default.country).to eq default_country end end # Regression test for https://github.com/spree/spree/issues/1142 - it "uses the first available country if :default_country_id is set to an invalid value" do - Spree::Config[:default_country_id] = "0" - expect(Spree::Address.build_default.country).to eq default_country + it "raises ActiveRecord::RecordNotFound if :default_country_iso is set to an invalid value" do + Spree::Config[:default_country_iso] = "00" + expect { + Spree::Address.build_default.country + }.to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/core/spec/models/spree/app_configuration_spec.rb b/core/spec/models/spree/app_configuration_spec.rb index 37ede21a976..41960ccfc3c 100644 --- a/core/spec/models/spree/app_configuration_spec.rb +++ b/core/spec/models/spree/app_configuration_spec.rb @@ -24,4 +24,10 @@ subject { prefs.stock } it { is_expected.to be_a Spree::Core::StockConfiguration } end + + describe '@default_country_iso_code' do + it 'is the USA by default' do + expect(prefs[:default_country_iso]).to eq("US") + end + end end diff --git a/core/spec/models/spree/country_spec.rb b/core/spec/models/spree/country_spec.rb new file mode 100644 index 00000000000..24753382c04 --- /dev/null +++ b/core/spec/models/spree/country_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe Spree::Country, type: :model do + describe '.default' do + before do + create(:country, iso: "DE", id: 1) + create(:country, id: 2) + end + + subject(:default_country) { described_class.default } + + context 'with the configuration setting an existing legacy default country ID' do + before do + Spree::Config[:default_country_id] = 2 + end + + it 'emits a deprecation warning' do + expect(ActiveSupport::Deprecation).to receive(:warn) + default_country + end + + it 'is the country with that ID' do + expect(default_country).to eq(Spree::Country.find(2)) + end + end + + context 'with the configuration setting a non-existing legacy default country ID' do + before do + Spree::Config[:default_country_id] = 0 + end + + it 'loads the country configured by the ISO code' do + expect(default_country).to eq(Spree::Country.find(2)) + end + end + + context 'with the configuration setting an existing ISO code' do + it 'is a country with the configurations ISO code' do + expect(described_class.default).to be_a(Spree::Country) + expect(described_class.default.iso).to eq('US') + end + end + + context 'with the configuration setting an non-existing ISO code' do + before { Spree::Config[:default_country_iso] = "ZZ" } + + it 'raises a Record not Found error' do + expect { described_class.default }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end +end diff --git a/frontend/spec/features/address_spec.rb b/frontend/spec/features/address_spec.rb index 8892e5fccee..af53b348b78 100644 --- a/frontend/spec/features/address_spec.rb +++ b/frontend/spec/features/address_spec.rb @@ -22,7 +22,7 @@ let!(:canada) { create(:country, name: "Canada", states_required: true, iso: "CA") } let!(:uk) { create(:country, name: "United Kingdom", states_required: true, iso: "GB") } - before { Spree::Config[:default_country_id] = uk.id } + before { Spree::Config[:default_country_iso] = uk.iso } context "but has no state" do it "shows the state input field" do @@ -47,7 +47,7 @@ end context "user changes to country without states required" do - let!(:france) { create(:country, name: "France", states_required: false, iso: "FRA") } + let!(:france) { create(:country, name: "France", states_required: false, iso: "FR") } it "clears the state name" do click_button "Checkout" @@ -63,7 +63,7 @@ end context "country does not require state", js: true do - let!(:france) { create(:country, name: "France", states_required: false, iso: "FRA") } + let!(:france) { create(:country, name: "France", states_required: false, iso: "FR") } it "shows a disabled state input field" do click_button "Checkout"