From ba859111de2e6ad2c355c18e7e4dc26e75e98ff2 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 10:17:30 +0100 Subject: [PATCH] Merge decorators with original files brought from spree --- app/models/spree/address.rb | 32 +++++++ app/models/spree/address_decorator.rb | 38 -------- app/models/spree/shipping_method.rb | 78 ++++++++++++++-- app/models/spree/shipping_method_decorator.rb | 90 ------------------- spec/models/spree/shipping_method_spec.rb | 19 ---- 5 files changed, 103 insertions(+), 154 deletions(-) delete mode 100644 app/models/spree/address_decorator.rb delete mode 100644 app/models/spree/shipping_method_decorator.rb diff --git a/app/models/spree/address.rb b/app/models/spree/address.rb index 1285ca3380..af7643dd31 100644 --- a/app/models/spree/address.rb +++ b/app/models/spree/address.rb @@ -1,8 +1,11 @@ module Spree class Address < ActiveRecord::Base + include AddressDisplay + belongs_to :country, class_name: "Spree::Country" belongs_to :state, class_name: "Spree::State" + has_one :enterprise, dependent: :restrict_with_exception has_many :shipments validates :firstname, :lastname, :address1, :city, :country, presence: true @@ -11,8 +14,13 @@ module Spree validate :state_validate + after_save :touch_enterprise + alias_attribute :first_name, :firstname alias_attribute :last_name, :lastname + delegate :name, to: :state, prefix: true, allow_nil: true + + geocoded_by :geocode_address def self.default country = Spree::Country.find(Spree::Config[:default_country_id]) rescue Spree::Country.first @@ -74,6 +82,22 @@ module Spree } end + def geocode_address + render_address([address1, address2, zipcode, city, country.andand.name, state.andand.name]) + end + + def full_address + render_address([address1, address2, city, zipcode, state.andand.name]) + end + + def address_part1 + render_address([address1, address2]) + end + + def address_part2 + render_address([city, zipcode, state.andand.name]) + end + private def require_phone? true @@ -119,5 +143,13 @@ module Spree # ensure at least one state field is populated errors.add :state, :blank if state.blank? && state_name.blank? end + + def touch_enterprise + enterprise.andand.touch + end + + def render_address(parts) + parts.select(&:present?).join(', ') + end end end diff --git a/app/models/spree/address_decorator.rb b/app/models/spree/address_decorator.rb deleted file mode 100644 index a8b1799e54..0000000000 --- a/app/models/spree/address_decorator.rb +++ /dev/null @@ -1,38 +0,0 @@ -Spree::Address.class_eval do - include AddressDisplay - - has_one :enterprise, dependent: :restrict_with_exception - belongs_to :country, class_name: "Spree::Country" - - after_save :touch_enterprise - - geocoded_by :geocode_address - - delegate :name, to: :state, prefix: true, allow_nil: true - - def geocode_address - render_address([address1, address2, zipcode, city, country.andand.name, state.andand.name]) - end - - def full_address - render_address([address1, address2, city, zipcode, state.andand.name]) - end - - def address_part1 - render_address([address1, address2]) - end - - def address_part2 - render_address([city, zipcode, state.andand.name]) - end - - private - - def touch_enterprise - enterprise.andand.touch - end - - def render_address(parts) - parts.select(&:present?).join(', ') - end -end diff --git a/app/models/spree/shipping_method.rb b/app/models/spree/shipping_method.rb index 6d610c9159..30aee7f6ca 100644 --- a/app/models/spree/shipping_method.rb +++ b/app/models/spree/shipping_method.rb @@ -3,30 +3,58 @@ module Spree include Spree::Core::CalculatedAdjustments DISPLAY = [:both, :front_end, :back_end] + acts_as_taggable + default_scope -> { where(deleted_at: nil) } has_many :shipments has_many :shipping_method_categories has_many :shipping_categories, through: :shipping_method_categories has_many :shipping_rates + has_many :distributor_shipping_methods + has_many :distributors, through: :distributor_shipping_methods, class_name: 'Enterprise', foreign_key: 'distributor_id' has_and_belongs_to_many :zones, :join_table => 'spree_shipping_methods_zones', :class_name => 'Spree::Zone', :foreign_key => 'shipping_method_id' validates :name, presence: true - + validate :distributor_validation validate :at_least_one_shipping_category + after_save :touch_distributors + + scope :managed_by, lambda { |user| + if user.has_spree_role?('admin') + where(nil) + else + joins(:distributors). + where('distributors_shipping_methods.distributor_id IN (?)', user.enterprises.select(&:id)). + select('DISTINCT spree_shipping_methods.*') + end + } + + scope :for_distributors, ->(distributors) { + non_unique_matches = unscoped.joins(:distributors).where(enterprises: { id: distributors }) + where(id: non_unique_matches.map(&:id)) + } + scope :for_distributor, lambda { |distributor| + joins(:distributors). + where('enterprises.id = ?', distributor) + } + + scope :by_name, -> { order('spree_shipping_methods.name ASC') } + scope :display_on_checkout, -> { where("spree_shipping_methods.display_on is null OR spree_shipping_methods.display_on = ''") } + def adjustment_label - Spree.t(:shipping) + I18n.t('shipping') end + # Here we allow checkout with shipping methods without zones (see issue #3928 for details) + # and also checkout with addresses outside of the zones of the selected shipping method + # This method could be used, like in Spree, to validate shipping method zones on checkout. def include?(address) - return false unless address - zones.any? do |zone| - zone.include?(address) - end + address.present? end def build_tracking_url(tracking) @@ -34,7 +62,7 @@ module Spree end def self.calculators - spree_calculators.send(model_name_without_spree_namespace).select{ |c| c < Spree::ShippingCalculator } + spree_calculators.send model_name_without_spree_namespace end # Some shipping methods are only meant to be set via backend @@ -42,6 +70,32 @@ module Spree self.display_on != "back_end" end + def has_distributor?(distributor) + distributors.include?(distributor) + end + + # Checks whether the shipping method is of delivery type, meaning that it + # requires the user to specify a ship address at checkout. + # + # @return [Boolean] + def delivery? + require_ship_address + end + + # Return the services (pickup, delivery) that different distributors provide, in the format: + # {distributor_id => {pickup: true, delivery: false}, ...} + def self.services + Hash[ + Spree::ShippingMethod. + joins(:distributor_shipping_methods). + group('distributor_id'). + select("distributor_id"). + select("BOOL_OR(spree_shipping_methods.require_ship_address = 'f') AS pickup"). + select("BOOL_OR(spree_shipping_methods.require_ship_address = 't') AS delivery"). + map { |sm| [sm.distributor_id.to_i, { pickup: sm.pickup, delivery: sm.delivery }] } + ] + end + private def at_least_one_shipping_category if self.shipping_categories.empty? @@ -56,5 +110,15 @@ module Spree def self.on_frontend_query "#{table_name}.display_on != 'back_end' OR #{table_name}.display_on IS NULL" end + + def touch_distributors + distributors.each do |distributor| + distributor.touch if distributor.persisted? + end + end + + def distributor_validation + validates_with DistributorsValidator + end end end diff --git a/app/models/spree/shipping_method_decorator.rb b/app/models/spree/shipping_method_decorator.rb deleted file mode 100644 index 58d42db120..0000000000 --- a/app/models/spree/shipping_method_decorator.rb +++ /dev/null @@ -1,90 +0,0 @@ -Spree::ShippingMethod.class_eval do - acts_as_taggable - - has_many :distributor_shipping_methods - has_many :distributors, through: :distributor_shipping_methods, class_name: 'Enterprise', foreign_key: 'distributor_id' - - after_save :touch_distributors - - validate :distributor_validation - - scope :managed_by, lambda { |user| - if user.has_spree_role?('admin') - where(nil) - else - joins(:distributors). - where('distributors_shipping_methods.distributor_id IN (?)', user.enterprises.select(&:id)). - select('DISTINCT spree_shipping_methods.*') - end - } - - scope :for_distributors, ->(distributors) { - non_unique_matches = unscoped.joins(:distributors).where(enterprises: { id: distributors }) - where(id: non_unique_matches.map(&:id)) - } - scope :for_distributor, lambda { |distributor| - joins(:distributors). - where('enterprises.id = ?', distributor) - } - - scope :by_name, -> { order('spree_shipping_methods.name ASC') } - scope :display_on_checkout, -> { where("spree_shipping_methods.display_on is null OR spree_shipping_methods.display_on = ''") } - - # Return the services (pickup, delivery) that different distributors provide, in the format: - # {distributor_id => {pickup: true, delivery: false}, ...} - def self.services - Hash[ - Spree::ShippingMethod. - joins(:distributor_shipping_methods). - group('distributor_id'). - select("distributor_id"). - select("BOOL_OR(spree_shipping_methods.require_ship_address = 'f') AS pickup"). - select("BOOL_OR(spree_shipping_methods.require_ship_address = 't') AS delivery"). - map { |sm| [sm.distributor_id.to_i, { pickup: sm.pickup, delivery: sm.delivery }] } - ] - end - - # This method is overriden so that we can remove the restriction added in Spree - # Spree restricts shipping method calculators to the ones that inherit from Spree::Shipping::ShippingCalculator - # Spree::Shipping::ShippingCalculator makes sure that calculators are able to handle packages and not orders as input - # This is not necessary in OFN because calculators in OFN are already customized to work with different types of input - def self.calculators - spree_calculators.send model_name_without_spree_namespace - end - - # This is bypassing the validation of shipping method zones on checkout - # It allows checkout using shipping methods without zones (see issue #3928 for details) - # and it allows checkout with addresses outside of the zones of the selected shipping method - def include?(address) - address.present? - end - - def has_distributor?(distributor) - distributors.include?(distributor) - end - - def adjustment_label - I18n.t('shipping') - end - - # Checks whether the shipping method is of delivery type, meaning that it - # requires the user to specify a ship address at checkout. Note this is - # a setting we added onto the +spree_shipping_methods+ table. - # - # @return [Boolean] - def delivery? - require_ship_address - end - - private - - def touch_distributors - distributors.each do |distributor| - distributor.touch if distributor.persisted? - end - end - - def distributor_validation - validates_with DistributorsValidator - end -end diff --git a/spec/models/spree/shipping_method_spec.rb b/spec/models/spree/shipping_method_spec.rb index 030ecc76c9..91798b302b 100644 --- a/spec/models/spree/shipping_method_spec.rb +++ b/spec/models/spree/shipping_method_spec.rb @@ -1,8 +1,5 @@ require 'spec_helper' -class DummyShippingCalculator < Spree::ShippingCalculator -end - module Spree describe ShippingMethod do it "is valid when built from factory" do @@ -123,22 +120,6 @@ module Spree end end - context 'calculators' do - let(:shipping_method){ create(:shipping_method) } - - it "Should reject calculators that don't inherit from Spree::ShippingCalculator" do - Spree::ShippingMethod.stub_chain(:spree_calculators, :shipping_methods).and_return([ - Spree::Calculator::Shipping::FlatPercentItemTotal, - Spree::Calculator::Shipping::PriceSack, - Spree::Calculator::DefaultTax, - DummyShippingCalculator # included as regression test for https://github.com/spree/spree/issues/3109 - ]) - - Spree::ShippingMethod.calculators.should == [Spree::Calculator::Shipping::FlatPercentItemTotal, Spree::Calculator::Shipping::PriceSack, DummyShippingCalculator ] - Spree::ShippingMethod.calculators.should_not == [Spree::Calculator::DefaultTax] - end - end - context "validations" do before { subject.valid? }