Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[spree_promo] Explicitly nest promotion classes. #1548

Closed
wants to merge 1 commit into from

2 participants

@JDutil
Collaborator

I've found a bug I'm attempting to fix within spree_product_groups. Unfortunately I was unable to run the specs due to:

/spree_product_groups/app/models/spree/promotion/rules/product_decorator.rb:1:in `<top (required)>': uninitialized constant Spree::Promotion::Rules::Product (NameError)

By explicitly nesting the classes and modules the initialization will properly define the classes / modules. This is similar to an issue we ran into with the spree_active_shipping gem described here: spree-contrib/spree_active_shipping#25

@JDutil
Collaborator

Found this error when writing specs to fix this issue spree-contrib/spree_product_groups#5

@radar radar closed this pull request from a commit
@JDutil JDutil Explicitly nest promotion classes.
Fixes #1548
36079b3
@radar radar closed this in 36079b3
@radar
Collaborator

Fixed with the above commit, which has been applied to 1-1-stable and master. Thank you!

@radar radar referenced this pull request from a commit
@JDutil JDutil Explicitly nest promotion classes.
Fixes #1548
730b0b1
@tvdeyen tvdeyen referenced this pull request from a commit in magiclabs/spree
@JDutil JDutil Explicitly nest promotion classes.
Fixes #1548
215dec3
@AsherBond AsherBond referenced this pull request from a commit
@JDutil JDutil Explicitly nest promotion classes.
Fixes #1548
9322a03
@trappist trappist referenced this pull request in spree-contrib/spree_product_groups
Closed

Explicitly nest classes #15

@stuartbates stuartbates referenced this pull request from a commit in stuartbates/spree_promo_slots
@trappist trappist Reimplement spree/spree#1548 for the same reason 39bd6bd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 16, 2012
  1. @JDutil
This page is out of date. Refresh to see the latest.
View
68 promo/app/models/spree/promotion/actions/create_adjustment.rb
@@ -1,43 +1,47 @@
module Spree
- class Promotion::Actions::CreateAdjustment < PromotionAction
- calculated_adjustments
+ class Promotion
+ module Actions
+ class CreateAdjustment < PromotionAction
+ calculated_adjustments
- delegate :eligible?, :to => :promotion
+ delegate :eligible?, :to => :promotion
- before_validation :ensure_action_has_calculator
+ before_validation :ensure_action_has_calculator
- def perform(options = {})
- return unless order = options[:order]
- # Nothing to do if the promotion is already associated with the order
- return if order.promotion_credit_exists?(promotion)
+ def perform(options = {})
+ return unless order = options[:order]
+ # Nothing to do if the promotion is already associated with the order
+ return if order.promotion_credit_exists?(promotion)
- order.adjustments.promotion.reload.clear
- order.update!
- create_adjustment("#{I18n.t(:promotion)} (#{promotion.name})", order, order)
- end
+ order.adjustments.promotion.reload.clear
+ order.update!
+ create_adjustment("#{I18n.t(:promotion)} (#{promotion.name})", order, order)
+ end
- # override of CalculatedAdjustments#create_adjustment so promotional
- # adjustments are added all the time. They will get their eligability
- # set to false if the amount is 0
- def create_adjustment(label, target, calculable, mandatory=false)
- amount = compute_amount(calculable)
- params = { :amount => amount,
- :source => calculable,
- :originator => self,
- :label => label,
- :mandatory => mandatory }
- target.adjustments.create(params, :without_protection => true)
- end
+ # override of CalculatedAdjustments#create_adjustment so promotional
+ # adjustments are added all the time. They will get their eligability
+ # set to false if the amount is 0
+ def create_adjustment(label, target, calculable, mandatory=false)
+ amount = compute_amount(calculable)
+ params = { :amount => amount,
+ :source => calculable,
+ :originator => self,
+ :label => label,
+ :mandatory => mandatory }
+ target.adjustments.create(params, :without_protection => true)
+ end
- # Ensure a negative amount which does not exceed the sum of the order's item_total and ship_total
- def compute_amount(calculable)
- [(calculable.item_total + calculable.ship_total), super.to_f.abs].min * -1
- end
+ # Ensure a negative amount which does not exceed the sum of the order's item_total and ship_total
+ def compute_amount(calculable)
+ [(calculable.item_total + calculable.ship_total), super.to_f.abs].min * -1
+ end
- private
- def ensure_action_has_calculator
- return if self.calculator
- self.calculator = Calculator::FlatPercentItemTotal.new
+ private
+ def ensure_action_has_calculator
+ return if self.calculator
+ self.calculator = Calculator::FlatPercentItemTotal.new
+ end
+ end
end
end
end
View
44 promo/app/models/spree/promotion/actions/create_line_items.rb
@@ -1,29 +1,33 @@
module Spree
- class Promotion::Actions::CreateLineItems < PromotionAction
- has_many :promotion_action_line_items, :foreign_key => 'promotion_action_id'
+ class Promotion
+ module Actions
+ class CreateLineItems < PromotionAction
+ has_many :promotion_action_line_items, :foreign_key => 'promotion_action_id'
- attr_accessor :line_items_string
+ attr_accessor :line_items_string
- def perform(options = {})
- return unless order = options[:order]
- promotion_action_line_items.each do |item|
- current_quantity = order.quantity_of(item.variant)
- if current_quantity < item.quantity
- order.add_variant(item.variant, item.quantity - current_quantity)
+ def perform(options = {})
+ return unless order = options[:order]
+ promotion_action_line_items.each do |item|
+ current_quantity = order.quantity_of(item.variant)
+ if current_quantity < item.quantity
+ order.add_variant(item.variant, item.quantity - current_quantity)
+ end
+ end
end
- end
- end
- def line_items_string
- promotion_action_line_items.map { |li| "#{li.variant_id}x#{li.quantity}" }.join(',')
- end
+ def line_items_string
+ promotion_action_line_items.map { |li| "#{li.variant_id}x#{li.quantity}" }.join(',')
+ end
- def line_items_string=(value)
- promotion_action_line_items.destroy_all
- value.to_s.split(',').each do |str|
- variant_id, quantity = str.split('x')
- if variant_id && quantity && variant = Variant.find_by_id(variant_id)
- promotion_action_line_items.create({:variant => variant, :quantity => quantity.to_i}, :without_protection => true)
+ def line_items_string=(value)
+ promotion_action_line_items.destroy_all
+ value.to_s.split(',').each do |str|
+ variant_id, quantity = str.split('x')
+ if variant_id && quantity && variant = Variant.find_by_id(variant_id)
+ promotion_action_line_items.create({:variant => variant, :quantity => quantity.to_i}, :without_protection => true)
+ end
+ end
end
end
end
View
12 promo/app/models/spree/promotion/rules/first_order.rb
@@ -1,8 +1,12 @@
module Spree
- class Promotion::Rules::FirstOrder < PromotionRule
- def eligible?(order, options = {})
- user = order.try(:user) || options[:user]
- !!(user && user.orders.complete.count == 0)
+ class Promotion
+ module Rules
+ class FirstOrder < PromotionRule
+ def eligible?(order, options = {})
+ user = order.try(:user) || options[:user]
+ !!(user && user.orders.complete.count == 0)
+ end
+ end
end
end
end
View
20 promo/app/models/spree/promotion/rules/item_total.rb
@@ -1,16 +1,20 @@
# A rule to limit a promotion to a specific user.
module Spree
- class Promotion::Rules::ItemTotal < PromotionRule
- preference :amount, :decimal, :default => 100.00
- preference :operator, :string, :default => '>'
+ class Promotion
+ module Rules
+ class ItemTotal < PromotionRule
+ preference :amount, :decimal, :default => 100.00
+ preference :operator, :string, :default => '>'
- attr_accessible :preferred_amount, :preferred_operator
+ attr_accessible :preferred_amount, :preferred_operator
- OPERATORS = ['gt', 'gte']
+ OPERATORS = ['gt', 'gte']
- def eligible?(order, options = {})
- item_total = order.line_items.map(&:amount).sum
- item_total.send(preferred_operator == 'gte' ? :>= : :>, BigDecimal.new(preferred_amount.to_s))
+ def eligible?(order, options = {})
+ item_total = order.line_items.map(&:amount).sum
+ item_total.send(preferred_operator == 'gte' ? :>= : :>, BigDecimal.new(preferred_amount.to_s))
+ end
+ end
end
end
end
View
46 promo/app/models/spree/promotion/rules/product.rb
@@ -2,32 +2,36 @@
# Can require all or any of the products to be present.
# Valid products either come from assigned product group or are assingned directly to the rule.
module Spree
- class Promotion::Rules::Product < PromotionRule
- has_and_belongs_to_many :products, :class_name => '::Spree::Product', :join_table => 'spree_products_promotion_rules', :foreign_key => 'promotion_rule_id'
+ class Promotion
+ module Rules
+ class Product < PromotionRule
+ has_and_belongs_to_many :products, :class_name => '::Spree::Product', :join_table => 'spree_products_promotion_rules', :foreign_key => 'promotion_rule_id'
- MATCH_POLICIES = %w(any all)
- preference :match_policy, :string, :default => MATCH_POLICIES.first
+ MATCH_POLICIES = %w(any all)
+ preference :match_policy, :string, :default => MATCH_POLICIES.first
- # scope/association that is used to test eligibility
- def eligible_products
- products
- end
+ # scope/association that is used to test eligibility
+ def eligible_products
+ products
+ end
- def eligible?(order, options = {})
- return true if eligible_products.empty?
- if preferred_match_policy == 'all'
- eligible_products.all? {|p| order.products.include?(p) }
- else
- order.products.any? {|p| eligible_products.include?(p) }
- end
- end
+ def eligible?(order, options = {})
+ return true if eligible_products.empty?
+ if preferred_match_policy == 'all'
+ eligible_products.all? {|p| order.products.include?(p) }
+ else
+ order.products.any? {|p| eligible_products.include?(p) }
+ end
+ end
- def product_ids_string
- product_ids.join(',')
- end
+ def product_ids_string
+ product_ids.join(',')
+ end
- def product_ids_string=(s)
- self.product_ids = s.to_s.split(',').map(&:strip)
+ def product_ids_string=(s)
+ self.product_ids = s.to_s.split(',').map(&:strip)
+ end
+ end
end
end
end
View
26 promo/app/models/spree/promotion/rules/user.rb
@@ -1,18 +1,22 @@
module Spree
- class Promotion::Rules::User < PromotionRule
- belongs_to :user
- has_and_belongs_to_many :users, :class_name => '::Spree::User', :join_table => 'spree_promotion_rules_users', :foreign_key => 'promotion_rule_id'
+ class Promotion
+ module Rules
+ class User < PromotionRule
+ belongs_to :user
+ has_and_belongs_to_many :users, :class_name => '::Spree::User', :join_table => 'spree_promotion_rules_users', :foreign_key => 'promotion_rule_id'
- def eligible?(order, options = {})
- users.none? or users.include?(order.user)
- end
+ def eligible?(order, options = {})
+ users.none? or users.include?(order.user)
+ end
- def user_ids_string
- user_ids.join(',')
- end
+ def user_ids_string
+ user_ids.join(',')
+ end
- def user_ids_string=(s)
- self.user_ids = s.to_s.split(',').map(&:strip)
+ def user_ids_string=(s)
+ self.user_ids = s.to_s.split(',').map(&:strip)
+ end
+ end
end
end
end
View
21 promo/app/models/spree/promotion/rules/user_logged_in.rb
@@ -1,15 +1,20 @@
module Spree
- class Promotion::Rules::UserLoggedIn < PromotionRule
- def eligible?(order, options = {})
+ class Promotion
+ module Rules
+ class UserLoggedIn < PromotionRule
- # this is tricky. We couldn't use any of the devise methods since we aren't in the controller.
- # we need to rely on the controller already having done this for us.
+ def eligible?(order, options = {})
+ # this is tricky. We couldn't use any of the devise methods since we aren't in the controller.
+ # we need to rely on the controller already having done this for us.
- # The thinking is that the controller should have some sense of what state
- # we should be in before firing events,
- # so the controller will have to set this field.
+ # The thinking is that the controller should have some sense of what state
+ # we should be in before firing events,
+ # so the controller will have to set this field.
- return options && options[:user_signed_in]
+ return options && options[:user_signed_in]
+ end
+
+ end
end
end
end
Something went wrong with that request. Please try again.