From fe67c01e57c41e29aba097eea40ac01315118047 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Fri, 14 May 2021 08:49:23 -0700 Subject: [PATCH 1/4] guard against no database, but not against .connected? Similar to b22ce43efe3f1e858161034fcba4c6831c63d3a4 --- config/application.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/application.rb b/config/application.rb index 45c68a66b3..57b5d277ab 100644 --- a/config/application.rb +++ b/config/application.rb @@ -76,10 +76,10 @@ module Openfoodnetwork Spree::Config['checkout_zone'] = ENV['CHECKOUT_ZONE'] Spree::Config['currency'] = ENV['CURRENCY'] - if ActiveRecord::Base.connected? && Spree::Country.table_exists? + begin country = Spree::Country.find_by(iso: ENV['DEFAULT_COUNTRY_CODE']) Spree::Config['default_country_id'] = country.id if country.present? - else + rescue ::ActiveRecord::StatementInvalid Spree::Config['default_country_id'] = 12 # Australia end end From 7df2915fbd228926a3556a8b3191b6990ede1af3 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Tue, 18 May 2021 12:14:01 -0700 Subject: [PATCH 2/4] add DefaultCountry service --- .../services/gmaps_geo.js.erb.coffee | 4 ++-- .../admin/enterprise_groups_controller.rb | 4 +--- .../admin/enterprises_controller.rb | 2 +- app/models/spree/address.rb | 2 +- app/models/spree/tax_rate.rb | 2 +- app/services/default_country.rb | 22 +++++++++++++++++++ app/views/admin/enterprises/new.html.haml | 2 +- app/views/checkout/_billing.html.haml | 2 +- .../checkout/_shipping_ship_address.html.haml | 2 +- .../registration/steps/_details.html.haml | 2 +- .../shared/_configuration_menu.html.haml | 4 ++-- db/default/countries.rb | 1 - .../admin/configuration/states_spec.rb | 2 +- spec/models/spree/address_spec.rb | 12 +++++----- spec/spec_helper.rb | 2 +- 15 files changed, 42 insertions(+), 23 deletions(-) create mode 100644 app/services/default_country.rb diff --git a/app/assets/javascripts/darkswarm/services/gmaps_geo.js.erb.coffee b/app/assets/javascripts/darkswarm/services/gmaps_geo.js.erb.coffee index 9b78ea4bcc..45b4628641 100644 --- a/app/assets/javascripts/darkswarm/services/gmaps_geo.js.erb.coffee +++ b/app/assets/javascripts/darkswarm/services/gmaps_geo.js.erb.coffee @@ -10,7 +10,7 @@ Darkswarm.service "GmapsGeo", -> # console.log "Error: #{status}" geocode: (address, callback) -> geocoder = new google.maps.Geocoder() - geocoder.geocode {'address': address, 'region': "<%= Spree::Country.find_by(id: Spree::Config[:default_country_id]).iso %>"}, callback + geocoder.geocode {'address': address, 'region': "<%= DefaultCountry.code %>"}, callback distanceBetween: (src, dst) -> google.maps.geometry.spherical.computeDistanceBetween @toLatLng(src), @toLatLng(dst) @@ -20,4 +20,4 @@ Darkswarm.service "GmapsGeo", -> if locatable.lat? locatable else - new google.maps.LatLng locatable.latitude, locatable.longitude \ No newline at end of file + new google.maps.LatLng locatable.latitude, locatable.longitude diff --git a/app/controllers/admin/enterprise_groups_controller.rb b/app/controllers/admin/enterprise_groups_controller.rb index 0691d9eeaa..bfd5cca831 100644 --- a/app/controllers/admin/enterprise_groups_controller.rb +++ b/app/controllers/admin/enterprise_groups_controller.rb @@ -28,9 +28,7 @@ module Admin def build_resource enterprise_group = super enterprise_group.address = Spree::Address.new - enterprise_group.address.country = Spree::Country.find_by( - id: Spree::Config[:default_country_id] - ) + enterprise_group.address.country = DefaultCountry.country enterprise_group end diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index db87019011..a65dc15a93 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -119,7 +119,7 @@ module Admin def build_resource enterprise = super enterprise.address ||= Spree::Address.new - enterprise.address.country ||= Spree::Country.find_by(id: Spree::Config[:default_country_id]) + enterprise.address.country ||= DefaultCountry.country enterprise end diff --git a/app/models/spree/address.rb b/app/models/spree/address.rb index 7a4307f2a1..45834510d7 100644 --- a/app/models/spree/address.rb +++ b/app/models/spree/address.rb @@ -24,7 +24,7 @@ module Spree def self.default country = begin - Spree::Country.find(Spree::Config[:default_country_id]) + DefaultCountry.country rescue StandardError Spree::Country.first end diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb index cce477b29d..f3f28495e6 100644 --- a/app/models/spree/tax_rate.rb +++ b/app/models/spree/tax_rate.rb @@ -50,7 +50,7 @@ module Spree category = TaxCategory.includes(:tax_rates).find_by(is_default: true) return 0 unless category - address ||= Address.new(country_id: Spree::Config[:default_country_id]) + address ||= Address.new(country_id: DefaultCountry.id) rate = category.tax_rates.detect { |tax_rate| tax_rate.zone.include? address }.try(:amount) rate || 0 diff --git a/app/services/default_country.rb b/app/services/default_country.rb new file mode 100644 index 0000000000..7c753cd569 --- /dev/null +++ b/app/services/default_country.rb @@ -0,0 +1,22 @@ +class DefaultCountry + def self.id + DefaultCountry.country.id + end + + def self.code + DefaultCountry.country.iso + end + + def self.country + Spree::Country.find_by(iso: ENV["DEFAULT_COUNTRY_CODE"]) || Spree::Country.first + end + + def self.id=(id) + ENV["DEFAULT_COUNTRY_CODE"] = Spree::Country.find(id).iso + end + + def self.code=(code) + country = Spree::Country.find_by(iso: code) || Spree::Country.first + ENV["DEFAULT_COUNTRY_CODE"] = country.iso + end +end diff --git a/app/views/admin/enterprises/new.html.haml b/app/views/admin/enterprises/new.html.haml index ec54e17900..4cd0297362 100644 --- a/app/views/admin/enterprises/new.html.haml +++ b/app/views/admin/enterprises/new.html.haml @@ -10,7 +10,7 @@ = "admin.enterprises" = admin_inject_available_countries(module: 'admin.enterprises') -= admin_inject_json "admin.enterprises", "defaultCountryID", Spree::Config[:default_country_id] += admin_inject_json "admin.enterprises", "defaultCountryID", DefaultCountry.id -# Form diff --git a/app/views/checkout/_billing.html.haml b/app/views/checkout/_billing.html.haml index 07b0d0f251..9be3ed6e45 100644 --- a/app/views/checkout/_billing.html.haml +++ b/app/views/checkout/_billing.html.haml @@ -38,7 +38,7 @@ = validated_input t(:postcode), "order.bill_address.zipcode" .small-6.columns.right - = validated_select t(:country), "order.bill_address.country_id", {}, {"ng-init" => "order.bill_address.country_id = order.bill_address.country_id || #{Spree::Config[:default_country_id]}", "ng-options" => "c.id as c.name for c in countries"} + = validated_select t(:country), "order.bill_address.country_id", {}, {"ng-init" => "order.bill_address.country_id = order.bill_address.country_id || #{DefaultCountry.id}", "ng-options" => "c.id as c.name for c in countries"} .row .small-12.columns.text-right diff --git a/app/views/checkout/_shipping_ship_address.html.haml b/app/views/checkout/_shipping_ship_address.html.haml index 1119886a47..ac399d544c 100644 --- a/app/views/checkout/_shipping_ship_address.html.haml +++ b/app/views/checkout/_shipping_ship_address.html.haml @@ -22,7 +22,7 @@ .small-6.columns = validated_input t(:postcode), "order.ship_address.zipcode" .small-6.columns.right - = validated_select t(:country), "order.ship_address.country_id", {}, {"ng-init" => "order.ship_address.country_id = order.ship_address.country_id || #{Spree::Config[:default_country_id]}", "ng-options" => "c.id as c.name for c in countries"} + = validated_select t(:country), "order.ship_address.country_id", {}, {"ng-init" => "order.ship_address.country_id = order.ship_address.country_id || #{DefaultCountry.id}", "ng-options" => "c.id as c.name for c in countries"} .row .small-6.columns = validated_input t(:phone), "order.ship_address.phone" diff --git a/app/views/registration/steps/_details.html.haml b/app/views/registration/steps/_details.html.haml index faabc3b965..27e56e9f64 100644 --- a/app/views/registration/steps/_details.html.haml +++ b/app/views/registration/steps/_details.html.haml @@ -53,7 +53,7 @@ .small-12.medium-8.large-8.columns .field %label{ for: 'enterprise_country' }= t(".country_field") - %select.chunky{ id: 'enterprise_country', name: 'country', required: true, ng: { init: "setDefaultCountry(#{Spree::Config[:default_country_id]})", model: 'enterprise.country', options: 'c as c.name for c in countries' } } + %select.chunky{ id: 'enterprise_country', name: 'country', required: true, ng: { init: "setDefaultCountry(#{DefaultCountry.id})", model: 'enterprise.country', options: 'c as c.name for c in countries' } } %span.error{ ng: { show: "details.country.$error.required && submitted" } } = t(".country_field_error") diff --git a/app/views/spree/admin/shared/_configuration_menu.html.haml b/app/views/spree/admin/shared/_configuration_menu.html.haml index d9d97f7915..b9b0167497 100644 --- a/app/views/spree/admin/shared/_configuration_menu.html.haml +++ b/app/views/spree/admin/shared/_configuration_menu.html.haml @@ -11,8 +11,8 @@ = configurations_sidebar_menu_item Spree.t(:tax_settings), edit_admin_tax_settings_path = configurations_sidebar_menu_item Spree.t(:zones), admin_zones_path = configurations_sidebar_menu_item Spree.t(:countries), admin_countries_path - - if Spree::Config[:default_country_id] - = configurations_sidebar_menu_item Spree.t(:states), admin_country_states_path(Spree::Config[:default_country_id]) + - if DefaultCountry.id + = configurations_sidebar_menu_item Spree.t(:states), admin_country_states_path(DefaultCountry.id) = configurations_sidebar_menu_item Spree.t(:payment_methods), admin_payment_methods_path = configurations_sidebar_menu_item Spree.t(:taxonomies), admin_taxonomies_path = configurations_sidebar_menu_item Spree.t(:shipping_methods), admin_shipping_methods_path diff --git a/db/default/countries.rb b/db/default/countries.rb index 1df48ed11b..4cc00cb560 100644 --- a/db/default/countries.rb +++ b/db/default/countries.rb @@ -227,4 +227,3 @@ Spree::Country.create!([ { name: "Saint Kitts and Nevis", iso3: "KNA", iso: "KN", iso_name: "SAINT KITTS AND NEVIS", numcode: "659" }, { name: "Serbia", iso3: "SRB", iso: "RS", "iso_name" => "SERBIA", numcode: "999" } ]) -Spree::Config[:default_country_id] = Spree::Country.find_by(iso: ENV.fetch("DEFAULT_COUNTRY_CODE", "US")).id diff --git a/spec/features/admin/configuration/states_spec.rb b/spec/features/admin/configuration/states_spec.rb index 90292a1708..fc8d025fcd 100755 --- a/spec/features/admin/configuration/states_spec.rb +++ b/spec/features/admin/configuration/states_spec.rb @@ -11,7 +11,7 @@ describe "States" do before(:each) do login_as_admin @hungary = Spree::Country.create!(name: "Hungary", iso_name: "Hungary") - Spree::Config[:default_country_id] = country.id + DefaultCountry.code = country.iso end # TODO: For whatever reason, rendering of the states page takes a non-trivial amount of time diff --git a/spec/models/spree/address_spec.rb b/spec/models/spree/address_spec.rb index 12ff129676..f57f04d25e 100644 --- a/spec/models/spree/address_spec.rb +++ b/spec/models/spree/address_spec.rb @@ -145,21 +145,21 @@ describe Spree::Address do context ".default" do before do - @default_country_id = Spree::Config[:default_country_id] + @default_country_id = DefaultCountry.id new_country = create(:country) - Spree::Config[:default_country_id] = new_country.id + DefaultCountry.code = new_country.iso end after do - Spree::Config[:default_country_id] = @default_country_id + DefaultCountry.id = @default_country_id end - it "sets up a new record with Spree::Config[:default_country_id]" do - expect(Spree::Address.default.country).to eq Spree::Country.find(Spree::Config[:default_country_id]) + it "sets up a new record the default country" do + expect(Spree::Address.default.country).to eq DefaultCountry.country end # Regression test for #1142 it "uses the first available country if :default_country_id is set to an invalid value" do - Spree::Config[:default_country_id] = "0" + DefaultCountry.code = "notacountry" expect(Spree::Address.default.country).to eq Spree::Country.first end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b483c52be9..9cf2365e84 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -169,7 +169,7 @@ RSpec.configure do |config| # Geocoding config.before(:each) { allow_any_instance_of(Spree::Address).to receive(:geocode).and_return([1, 1]) } - default_country_id = Spree::Config[:default_country_id] + default_country_id = DefaultCountry.id checkout_zone = Spree::Config[:checkout_zone] currency = Spree::Config[:currency] # Ensure we start with consistent config settings From 45ad4bbcf102a7579b82f80c08311621969fed9b Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 19 May 2021 09:33:02 -0700 Subject: [PATCH 3/4] remove setting of Spree::Config country code --- config/application.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/config/application.rb b/config/application.rb index 57b5d277ab..279927317d 100644 --- a/config/application.rb +++ b/config/application.rb @@ -75,13 +75,6 @@ module Openfoodnetwork initializer 'ofn.spree_locale_settings', before: 'spree.promo.environment' do |app| Spree::Config['checkout_zone'] = ENV['CHECKOUT_ZONE'] Spree::Config['currency'] = ENV['CURRENCY'] - - begin - country = Spree::Country.find_by(iso: ENV['DEFAULT_COUNTRY_CODE']) - Spree::Config['default_country_id'] = country.id if country.present? - rescue ::ActiveRecord::StatementInvalid - Spree::Config['default_country_id'] = 12 # Australia - end end # Register Spree calculators From e73584fef74927f96bf50d5200a94d65b7b9e0d2 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 19 May 2021 09:42:52 -0700 Subject: [PATCH 4/4] remove setters from DefaultCountry service --- app/services/default_country.rb | 13 ++--------- .../admin/configuration/states_spec.rb | 6 +++-- spec/models/spree/address_spec.rb | 22 +++++++++---------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/app/services/default_country.rb b/app/services/default_country.rb index 7c753cd569..c805e261b0 100644 --- a/app/services/default_country.rb +++ b/app/services/default_country.rb @@ -1,22 +1,13 @@ class DefaultCountry def self.id - DefaultCountry.country.id + country.id end def self.code - DefaultCountry.country.iso + country.iso end def self.country Spree::Country.find_by(iso: ENV["DEFAULT_COUNTRY_CODE"]) || Spree::Country.first end - - def self.id=(id) - ENV["DEFAULT_COUNTRY_CODE"] = Spree::Country.find(id).iso - end - - def self.code=(code) - country = Spree::Country.find_by(iso: code) || Spree::Country.first - ENV["DEFAULT_COUNTRY_CODE"] = country.iso - end end diff --git a/spec/features/admin/configuration/states_spec.rb b/spec/features/admin/configuration/states_spec.rb index fc8d025fcd..10e87ceb88 100755 --- a/spec/features/admin/configuration/states_spec.rb +++ b/spec/features/admin/configuration/states_spec.rb @@ -10,8 +10,10 @@ describe "States" do before(:each) do login_as_admin - @hungary = Spree::Country.create!(name: "Hungary", iso_name: "Hungary") - DefaultCountry.code = country.iso + @hungary = Spree::Country.create!(name: "Hungary", iso_name: "Hungary", iso: "HU") + + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DEFAULT_COUNTRY_CODE").and_return("HU") end # TODO: For whatever reason, rendering of the states page takes a non-trivial amount of time diff --git a/spec/models/spree/address_spec.rb b/spec/models/spree/address_spec.rb index f57f04d25e..f334392657 100644 --- a/spec/models/spree/address_spec.rb +++ b/spec/models/spree/address_spec.rb @@ -144,23 +144,21 @@ describe Spree::Address do end context ".default" do - before do - @default_country_id = DefaultCountry.id - new_country = create(:country) - DefaultCountry.code = new_country.iso - end - - after do - DefaultCountry.id = @default_country_id - end it "sets up a new record the default country" do expect(Spree::Address.default.country).to eq DefaultCountry.country end # Regression test for #1142 - it "uses the first available country if :default_country_id is set to an invalid value" do - DefaultCountry.code = "notacountry" - expect(Spree::Address.default.country).to eq Spree::Country.first + + context "The default country code is set to an invalid value" do + before do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DEFAULT_COUNTRY_CODE").and_return("notacountry") + end + + it "uses the first available country" do + expect(Spree::Address.default.country).to eq Spree::Country.first + end end end