diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 378129593e..8d06d3cfad 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -674,7 +674,6 @@ Layout/SpaceAroundOperators: - 'app/controllers/checkout_controller.rb' - 'app/helpers/admin/business_model_configuration_helper.rb' - 'app/jobs/update_billable_periods.rb' - - 'app/models/spree/address_decorator.rb' - 'app/overrides/remove_search_bar.rb' - 'app/overrides/remove_side_bar.rb' - 'app/overrides/replace_shipping_address_form_with_distributor_details.rb' @@ -1441,7 +1440,6 @@ Rails/HasManyOrHasOneDependent: - 'app/models/customer.rb' - 'app/models/enterprise.rb' - 'app/models/order_cycle.rb' - - 'app/models/spree/address_decorator.rb' - 'app/models/spree/adjustment_decorator.rb' - 'app/models/spree/order_decorator.rb' - 'app/models/spree/payment_method_decorator.rb' @@ -1955,7 +1953,6 @@ Style/HashSyntax: - 'app/models/open_food_network/calculator/weight.rb' - 'app/models/order_cycle.rb' - 'app/models/product_distribution.rb' - - 'app/models/spree/address_decorator.rb' - 'app/models/spree/adjustment_decorator.rb' - 'app/models/spree/classification_decorator.rb' - 'app/models/spree/gateway/migs.rb' @@ -2313,7 +2310,6 @@ Style/RedundantSelf: - 'app/models/open_food_network/calculator/weight.rb' - 'app/models/order_cycle.rb' - 'app/models/producer_property.rb' - - 'app/models/spree/address_decorator.rb' - 'app/models/spree/calculator/flat_percent_item_total_decorator.rb' - 'app/models/spree/calculator/flexi_rate_decorator.rb' - 'app/models/spree/calculator/per_item_decorator.rb' diff --git a/app/models/spree/address_decorator.rb b/app/models/spree/address_decorator.rb index db092f5eb2..ff7c1f0665 100644 --- a/app/models/spree/address_decorator.rb +++ b/app/models/spree/address_decorator.rb @@ -1,35 +1,27 @@ Spree::Address.class_eval do - has_one :enterprise + has_one :enterprise, dependent: :restrict belongs_to :country, class_name: "Spree::Country" after_save :touch_enterprise geocoded_by :geocode_address - delegate :name, :to => :state, :prefix => true, :allow_nil => true + delegate :name, to: :state, prefix: true, allow_nil: true def geocode_address - geocode_address = [address1, address2, zipcode, city, country.andand.name, state.andand.name] - filtered_address = geocode_address.select{ |field| !field.nil? && field != '' } - filtered_address.compact.join(', ') + render_address([address1, address2, zipcode, city, country.andand.name, state.andand.name]) end def full_address - full_address = [address1, address2, city, zipcode, state.andand.name] - filtered_address = full_address.select{ |field| !field.nil? && field != '' } - filtered_address.compact.join(', ') + render_address([address1, address2, city, zipcode, state.andand.name]) end def address_part1 - address_part1 = [address1, address2] - filtered_address = address_part1.select{ |field| !field.nil? && field != '' } - filtered_address.compact.join(', ') + render_address([address1, address2]) end def address_part2 - address_part2= [city, zipcode, state.andand.name] - filtered_address = address_part2.select{ |field| !field.nil? && field != '' } - filtered_address.compact.join(', ') + render_address([city, zipcode, state.andand.name]) end private @@ -38,26 +30,7 @@ Spree::Address.class_eval do enterprise.andand.touch end - # We have a hard-to-track-down bug around invalid addresses with all-nil fields finding - # their way into the database. I don't know what the source of them is, so this patch - # is designed to track them down. - # This is intended to be a temporary investigative measure, and should be removed from the - # code base shortly. If it's past 17-10-2013, take it out. - # - #-- Rohan, 17-9-2913 - def create - if self.zipcode.nil? - Bugsnag.notify RuntimeError.new('Creating a Spree::Address with nil values') - end - - super - end - - def update(attribute_names = @attributes.keys) - if self.zipcode.nil? - Bugsnag.notify RuntimeError.new('Updating a Spree::Address with nil values') - end - - super(attribute_names) + def render_address(parts) + parts.select(&:present?).join(', ') end end diff --git a/config/initializers/geocoder.rb b/config/initializers/geocoder.rb new file mode 100644 index 0000000000..35b5510d5e --- /dev/null +++ b/config/initializers/geocoder.rb @@ -0,0 +1,6 @@ +# Google requires an API key with a billing account to use their API. +# The key is stored in config/application.yml. +Geocoder.configure( + use_https: true, + api_key: ENV.fetch('GOOGLE_MAPS_API_KEY', nil) +) diff --git a/spec/models/spree/addresses_spec.rb b/spec/models/spree/addresses_spec.rb index f453218610..5d78cadcf2 100644 --- a/spec/models/spree/addresses_spec.rb +++ b/spec/models/spree/addresses_spec.rb @@ -1,31 +1,44 @@ require 'spec_helper' describe Spree::Address do + let(:address) { build(:address) } + let(:enterprise_address) { build(:address, enterprise: build(:enterprise)) } + describe "associations" do - it { should have_one(:enterprise) } + it { is_expected.to have_one(:enterprise) } end describe "delegation" do - it { should delegate(:name).to(:state).with_prefix } + it { is_expected.to delegate(:name).to(:state).with_prefix } + end + + describe "destroy" do + it "can be deleted" do + expect { address.destroy }.to_not raise_error + end + + it "cannot be deleted with associated enterprise" do + expect do + enterprise_address.destroy + end.to raise_error ActiveRecord::DeleteRestrictionError + end end describe "geocode address" do - let(:address) { FactoryBot.build(:address) } - it "should include address1, address2, zipcode, city, state and country" do - address.geocode_address.should include(address.address1) - address.geocode_address.should include(address.address2) - address.geocode_address.should include(address.zipcode) - address.geocode_address.should include(address.city) - address.geocode_address.should include(address.state.name) - address.geocode_address.should include(address.country.name) + expect(address.geocode_address).to include(address.address1) + expect(address.geocode_address).to include(address.address2) + expect(address.geocode_address).to include(address.zipcode) + expect(address.geocode_address).to include(address.city) + expect(address.geocode_address).to include(address.state.name) + expect(address.geocode_address).to include(address.country.name) end it "should not include empty fields" do address.address2 = nil address.city = "" - address.geocode_address.split(',').length.should eql(4) + expect(address.geocode_address.split(',').length).to eql(4) end end @@ -33,40 +46,27 @@ describe Spree::Address do let(:address) { FactoryBot.build(:address) } it "should include address1, address2, zipcode, city and state" do - address.full_address.should include(address.address1) - address.full_address.should include(address.address2) - address.full_address.should include(address.zipcode) - address.full_address.should include(address.city) - address.full_address.should include(address.state.name) - address.full_address.should_not include(address.country.name) + expect(address.full_address).to include(address.address1) + expect(address.full_address).to include(address.address2) + expect(address.full_address).to include(address.zipcode) + expect(address.full_address).to include(address.city) + expect(address.full_address).to include(address.state.name) + expect(address.full_address).not_to include(address.country.name) end it "should not include empty fields" do address.address2 = nil address.city = "" - address.full_address.split(',').length.should eql(3) + expect(address.full_address.split(',').length).to eql(3) end end describe "setters" do it "lets us set a country" do - expect { Spree::Address.new.country = "A country" }.to raise_error ActiveRecord::AssociationTypeMismatch - end - end - - describe "notifying bugsnag when saved with missing data" do - it "notifies on create" do - Bugsnag.should_receive(:notify) - a = Spree::Address.new zipcode: nil - a.save validate: false - end - - it "notifies on update" do - Bugsnag.should_receive(:notify) - a = create(:address) - a.zipcode = nil - a.save validate: false + expect do + Spree::Address.new.country = "A country" + end.to raise_error ActiveRecord::AssociationTypeMismatch end end end