From 50eb8f31f209372ba7370ee7d60371e69d186822 Mon Sep 17 00:00:00 2001 From: Neal Chambers Date: Fri, 22 Sep 2023 16:29:08 +0900 Subject: [PATCH 1/2] Remove Deprecated delegate_belongs_to --- app/models/application_record.rb | 1 - app/models/product_import/entry_validator.rb | 12 ++- app/models/spree/gateway.rb | 3 +- app/models/spree/variant.rb | 10 ++- lib/spree/core.rb | 1 - lib/spree/core/delegate_belongs_to.rb | 92 -------------------- 6 files changed, 16 insertions(+), 103 deletions(-) delete mode 100644 lib/spree/core/delegate_belongs_to.rb diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 7870e01a56f..e54fcb49aac 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class ApplicationRecord < ActiveRecord::Base - include DelegateBelongsTo include Spree::Core::Permalinks include Spree::Preferences::Preferable include Searchable diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index eb15aebb506..9166ae10ac9 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -67,10 +67,15 @@ def enterprise_field end def mark_as_new_variant(entry, product_id) + # Variant needs a product. Product needs to be assigned first in order for + # delegate to work. name= will fail otherwise. new_variant = Spree::Variant.new( - entry.assignable_attributes.except('id', 'product_id', 'on_hand', 'on_demand', - 'variant_unit', 'variant_unit_name', - 'variant_unit_scale', 'primary_taxon_id') + { product_id: } + .merge( + entry.assignable_attributes.except('id', 'product_id', 'on_hand', 'on_demand', + 'variant_unit', 'variant_unit_name', + 'variant_unit_scale', 'primary_taxon_id') + ) ) new_variant.save if new_variant.persisted? @@ -82,7 +87,6 @@ def mark_as_new_variant(entry, product_id) end end - new_variant.product_id = product_id check_on_hand_nil(entry, new_variant) if new_variant.valid? diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index b205cfae699..05d3e8abafb 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -1,14 +1,13 @@ # frozen_string_literal: true require 'concerns/payment_method_distributors' -require 'spree/core/delegate_belongs_to' module Spree class Gateway < PaymentMethod acts_as_taggable include PaymentMethodDistributors - delegate_belongs_to :provider, :authorize, :purchase, :capture, :void, :credit + delegate :authorize, :purchase, :capture, :void, :credit, to: :provider validates :name, :type, presence: true diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 5e706c801ed..3ca3317aa82 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -31,7 +31,7 @@ class Variant < ApplicationRecord belongs_to :tax_category, class_name: 'Spree::TaxCategory' belongs_to :shipping_category, class_name: 'Spree::ShippingCategory', optional: false - delegate_belongs_to :product, :name, :description, :meta_keywords + delegate :name, :name=, :description, :description=, :meta_keywords, to: :product has_many :inventory_units, inverse_of: :variant has_many :line_items, inverse_of: :variant @@ -51,8 +51,8 @@ class Variant < ApplicationRecord has_many :prices, class_name: 'Spree::Price', dependent: :destroy - delegate_belongs_to :default_price, :display_price, :display_amount, - :price, :price=, :currency + delegate :display_price, :display_amount, :price, :price=, :currency, :currency=, + to: :find_or_build_default_price has_many :exchange_variants has_many :exchanges, through: :exchange_variants @@ -221,6 +221,10 @@ def save_default_price default_price.save if default_price && (default_price.changed? || default_price.new_record?) end + def find_or_build_default_price + default_price || build_default_price + end + def set_cost_currency self.cost_currency = Spree::Config[:currency] if cost_currency.blank? end diff --git a/lib/spree/core.rb b/lib/spree/core.rb index 3daa1967004..4c39461913f 100644 --- a/lib/spree/core.rb +++ b/lib/spree/core.rb @@ -31,7 +31,6 @@ def self.config require 'spree/i18n' require 'spree/money' -require 'spree/core/delegate_belongs_to' require 'spree/core/permalinks' require 'spree/core/token_resource' require 'spree/core/product_duplicator' diff --git a/lib/spree/core/delegate_belongs_to.rb b/lib/spree/core/delegate_belongs_to.rb deleted file mode 100644 index fafe97392a5..00000000000 --- a/lib/spree/core/delegate_belongs_to.rb +++ /dev/null @@ -1,92 +0,0 @@ -# frozen_string_literal: true - -## -# Creates methods on object which delegate to an association proxy. -# see delegate_belongs_to for two uses -# -# Todo - integrate with ActiveRecord::Dirty to make sure changes to delegate object are noticed -# Should do -# class User < ActiveRecord::Base; delegate_belongs_to :contact, :firstname; end -# class Contact < ActiveRecord::Base; end -# u = User.first -# u.changed? # => false -# u.firstname = 'Bobby' -# u.changed? # => true -# -# Right now the second call to changed? would return false -# -# Todo - add has_one support. fairly straightforward addition -## -module DelegateBelongsTo - extend ActiveSupport::Concern - - module ClassMethods - @@default_rejected_delegate_columns = ['created_at', 'created_on', 'updated_at', - 'updated_on', 'lock_version', 'type', 'id', - 'position', 'parent_id', 'lft', 'rgt'] - mattr_accessor :default_rejected_delegate_columns - - ## - # Creates methods for accessing and setting attributes on an association. Uses same - # default list of attributes as delegates_to_association. - # delegate_belongs_to :contact - # delegate_belongs_to :contact, [:defaults] ## same as above, and useless - # delegate_belongs_to :contact, [:defaults, :address, :fullname], :class_name => 'VCard' - ## - def delegate_belongs_to(association, *attrs) - opts = attrs.extract_options! - initialize_association :belongs_to, association, opts - attrs = get_association_column_names(association) if attrs.empty? - attrs.concat get_association_column_names(association) if attrs.delete :defaults - attrs.each do |attr| - class_def attr do |*args| - if args.empty? - __send__(:delegator_for, association).__send__(attr) - else - __send__(:delegator_for, association).__send__(attr, *args) - end - end - class_def "#{attr}=" do |val| - __send__(:delegator_for, association).__send__("#{attr}=", val) - end - end - end - - protected - - def get_association_column_names(association, without_default_rejected_delegate_columns = true) - association_klass = reflect_on_association(association).klass - methods = association_klass.column_names - if without_default_rejected_delegate_columns - methods.reject!{ |x| default_rejected_delegate_columns.include?(x.to_s) } - end - methods - rescue StandardError - [] - end - - ## - # initialize_association :belongs_to, :contact - def initialize_association(type, association, opts = {}) - unless [:belongs_to].include?(type.to_s.to_sym) - raise 'Illegal or unimplemented association type.' - end - - __send__(type, association, **opts) if reflect_on_association(association).nil? - end - - private - - def class_def(name, method = nil, &) - class_eval { method.nil? ? define_method(name, &) : define_method(name, method) } - end - end - - def delegator_for(association) - if __send__(association).nil? - __send__("#{association}=", self.class.reflect_on_association(association).klass.new) - end - __send__(association) - end - protected :delegator_for -end From 91b251acd4a56e6cb0c7cebd455651baba49366e Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 26 Sep 2023 10:56:44 +1000 Subject: [PATCH 2/2] Refactor --- app/models/product_import/entry_validator.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 9166ae10ac9..1e4e6be649f 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -67,16 +67,14 @@ def enterprise_field end def mark_as_new_variant(entry, product_id) + variant_attributes = entry.assignable_attributes.except( + 'id', 'product_id', 'on_hand', 'on_demand', 'variant_unit', 'variant_unit_name', + 'variant_unit_scale', 'primary_taxon_id' + ) # Variant needs a product. Product needs to be assigned first in order for # delegate to work. name= will fail otherwise. - new_variant = Spree::Variant.new( - { product_id: } - .merge( - entry.assignable_attributes.except('id', 'product_id', 'on_hand', 'on_demand', - 'variant_unit', 'variant_unit_name', - 'variant_unit_scale', 'primary_taxon_id') - ) - ) + new_variant = Spree::Variant.new(product_id:, **variant_attributes) + new_variant.save if new_variant.persisted? if entry.attributes['on_demand'].present?