diff --git a/promotions/MIGRATING.md b/promotions/MIGRATING.md index 84ca018c385..b83ceff1654 100644 --- a/promotions/MIGRATING.md +++ b/promotions/MIGRATING.md @@ -96,10 +96,14 @@ If you have custom promotion rules or actions, you need to create new conditions In our experience, using the three actions can do almost all the things necessary, since they are customizable using calculators. -Rules share a lot of the previous API. If you make use of `#actionable?`, you might want to migrate your rule to be a line-item level rule: +Rules share a lot of the previous API. If you make use of `#actionable?`, you might want to migrate your rule to be a combined order and line-item level rule: ```rb class MyRule < Spree::PromotionRule + def eligible?(order) + order.total > 100 + end + def actionable?(promotable) promotable.quantity > 3 end @@ -110,10 +114,12 @@ would become: ```rb class MyCondition < SolidusPromotions::Condition - include LineItemLevelCondition + def order_eligible?(order, _options = {}) + order.total > 100 + end - def eligible?(promotable) - promotable.quantity > 3 + def line_item_eligible?(line_item, _options = {}) + line_item.quantity > 3 end end ``` diff --git a/promotions/app/models/concerns/solidus_promotions/conditions/line_item_applicable_order_level_condition.rb b/promotions/app/models/concerns/solidus_promotions/conditions/line_item_applicable_order_level_condition.rb index 229a3a73d83..ab5f1954438 100644 --- a/promotions/app/models/concerns/solidus_promotions/conditions/line_item_applicable_order_level_condition.rb +++ b/promotions/app/models/concerns/solidus_promotions/conditions/line_item_applicable_order_level_condition.rb @@ -8,11 +8,7 @@ def self.included(klass) end def applicable?(promotable) - promotable.is_a?(Spree::Order) || preferred_line_item_applicable && promotable.is_a?(Spree::LineItem) - end - - def eligible?(promotable) - send(:"#{promotable.class.name.demodulize.underscore}_eligible?", promotable) + promotable.is_a?(Spree::LineItem) ? preferred_line_item_applicable && super : super end def level diff --git a/promotions/app/models/concerns/solidus_promotions/conditions/line_item_level_condition.rb b/promotions/app/models/concerns/solidus_promotions/conditions/line_item_level_condition.rb index de5f4063910..d23bda506f9 100644 --- a/promotions/app/models/concerns/solidus_promotions/conditions/line_item_level_condition.rb +++ b/promotions/app/models/concerns/solidus_promotions/conditions/line_item_level_condition.rb @@ -3,8 +3,20 @@ module SolidusPromotions module Conditions module LineItemLevelCondition - def applicable?(promotable) - promotable.is_a?(Spree::LineItem) + def self.included(base) + def base.method_added(method) + if method == :eligible? + Spree.deprecator.warn <<~MSG + Defining `eligible?` on a promotion along with including the `LineItemLevelCondition` module is deprecated. + Rename `eligible?` to `line_item_eligible?` and stop including the `LineItemLevelCondition` module. + MSG + define_method(:applicable?) do |promotable| + promotable.is_a?(Spree::LineItem) + end + end + + super + end end def level diff --git a/promotions/app/models/concerns/solidus_promotions/conditions/order_level_condition.rb b/promotions/app/models/concerns/solidus_promotions/conditions/order_level_condition.rb index 7f84cd874a7..7766dc3b582 100644 --- a/promotions/app/models/concerns/solidus_promotions/conditions/order_level_condition.rb +++ b/promotions/app/models/concerns/solidus_promotions/conditions/order_level_condition.rb @@ -3,8 +3,20 @@ module SolidusPromotions module Conditions module OrderLevelCondition - def applicable?(promotable) - promotable.is_a?(Spree::Order) + def self.included(base) + def base.method_added(method) + if method == :eligible? + Spree.deprecator.warn <<~MSG + Defining `eligible?` on a promotion along with including the `OrderLevelCondition` module is deprecated. + Rename `eligible?` to `order_eligible?` and stop including the `OrderLevelCondition` module. + MSG + define_method(:applicable?) do |promotable| + promotable.is_a?(Spree::Order) + end + end + + super + end end def level diff --git a/promotions/app/models/concerns/solidus_promotions/conditions/shipment_level_condition.rb b/promotions/app/models/concerns/solidus_promotions/conditions/shipment_level_condition.rb index 87a7dc049fb..a4c58c4b8e8 100644 --- a/promotions/app/models/concerns/solidus_promotions/conditions/shipment_level_condition.rb +++ b/promotions/app/models/concerns/solidus_promotions/conditions/shipment_level_condition.rb @@ -3,8 +3,20 @@ module SolidusPromotions module Conditions module ShipmentLevelCondition - def applicable?(promotable) - promotable.is_a?(Spree::Shipment) + def self.included(base) + def base.method_added(method) + if method == :eligible? + Spree.deprecator.warn <<~MSG + Defining `eligible?` on a promotion along with including the `ShipmentLevelCondition` module is deprecated. + Rename `eligible?` to `shipment_eligible?` and stop including the `ShipmentLevelCondition` module. + MSG + define_method(:applicable?) do |promotable| + promotable.is_a?(Spree::Shipment) + end + end + + super + end end def level diff --git a/promotions/app/models/solidus_promotions/condition.rb b/promotions/app/models/solidus_promotions/condition.rb index b1803de1605..62bd77830a3 100644 --- a/promotions/app/models/solidus_promotions/condition.rb +++ b/promotions/app/models/solidus_promotions/condition.rb @@ -71,58 +71,65 @@ def preload_relations # Determines if this condition can be applied to a given promotable object. # - # This method is typically implemented by including one of the level modules - # (OrderLevelCondition, LineItemLevelCondition, or LineItemApplicableOrderLevelCondition) - # rather than being overridden directly. - # # @param _promotable [Object] The object to check (e.g., Spree::Order, Spree::LineItem) # # @return [Boolean] true if this condition applies to the promotable type # - # @raise [NotImplementedError] if not implemented in subclass - # - # @example Order-level condition applicability + # @example Condition applicability # condition.applicable?(order) # => true # condition.applicable?(line_item) # => false - # - # @see OrderLevelCondition - # @see LineItemLevelCondition - # @see LineItemApplicableOrderLevelCondition - def applicable?(_promotable) - raise NotImplementedError, "applicable? should be implemented in a sub-class of SolidusPromotions::Condition" + def applicable?(promotable) + respond_to?(eligible_method_for(promotable)) end # Determines if the promotable object meets this condition's eligibility requirements. # - # This is the core method that implements the condition's logic. When the promotable - # is not eligible, this method should add errors to {#eligibility_errors} explaining why. + # This typically dispatches to a specific eligibility method defined on a subclass, such as + # `#order_eligible?` or `line_item_eligible?`. # # @param _promotable [Object] The object to evaluate (e.g., Spree::Order, Spree::LineItem) # @param _options [Hash] Additional options for eligibility checking # # @return [Boolean] true if the promotable meets the condition, false otherwise # - # @raise [NotImplementedError] if not implemented in subclass - # - # @example Order total condition - # def eligible?(order, _options = {}) - # if order.item_total < preferred_minimum - # eligibility_errors.add(:base, "Order total too low") - # end - # eligibility_errors.empty? - # end - # - # @example First order condition - # def eligible?(order, _options = {}) - # if order.user.orders.complete.count > 1 - # eligibility_errors.add(:base, "Not first order") - # end - # eligibility_errors.empty? - # end - # # @see #eligibility_errors - def eligible?(_promotable, _options = {}) - raise NotImplementedError, "eligible? should be implemented in a sub-class of SolidusPromotions::Condition" + def eligible?(promotable, ...) + if applicable?(promotable) + send(eligible_method_for(promotable), promotable, ...) + else + raise NotImplementedError, "Please implement #{eligible_method_for(promotable)} in your condition" + end + end + + def self.inherited(klass) + def klass.method_added(method_added) + if method_added == :eligible? + Spree.deprecator.warn <<~MSG + Please refactor `#{name}`. You're defining `eligible?`. Instead, define method for each type of promotable + that your condition can be applied to. For example: + ``` + class MyCondition < SolidusPromotions::Condition + def applicable?(promotable) + promotable.is_a?(Spree::Order) + end + + def eligible?(order) + order.total > 20 + end + ``` + can now become + ``` + class MyCondition < SolidusPromotions::Condition + def order_eligible?(order) + order.total > 20 + end + end + ``` + MSG + end + super + end + super end def level @@ -170,6 +177,13 @@ def updateable? private + # Generates the eligibility method name for a promotable + # + # @return [Symbol] the method name + def eligible_method_for(promotable) + :"#{promotable.class.name.demodulize.underscore}_eligible?" + end + # Validates that only one instance of this condition type exists per benefit. # # Prevents duplicate conditions of the same type from being added to a single benefit. diff --git a/promotions/app/models/solidus_promotions/conditions/first_order.rb b/promotions/app/models/solidus_promotions/conditions/first_order.rb index 1894606c1fd..5a676217d13 100644 --- a/promotions/app/models/solidus_promotions/conditions/first_order.rb +++ b/promotions/app/models/solidus_promotions/conditions/first_order.rb @@ -3,10 +3,12 @@ module SolidusPromotions module Conditions class FirstOrder < Condition + # TODO: Remove in Solidus 5 include OrderLevelCondition + attr_reader :user, :email - def eligible?(order, options = {}) + def order_eligible?(order, options = {}) @user = order.try(:user) || options[:user] @email = order.email diff --git a/promotions/app/models/solidus_promotions/conditions/first_repeat_purchase_since.rb b/promotions/app/models/solidus_promotions/conditions/first_repeat_purchase_since.rb index d561c7671d9..888c4197153 100644 --- a/promotions/app/models/solidus_promotions/conditions/first_repeat_purchase_since.rb +++ b/promotions/app/models/solidus_promotions/conditions/first_repeat_purchase_since.rb @@ -3,6 +3,7 @@ module SolidusPromotions module Conditions class FirstRepeatPurchaseSince < Condition + # TODO: Remove in Solidus 5 include OrderLevelCondition preference :days_ago, :integer, default: 365 @@ -12,7 +13,7 @@ class FirstRepeatPurchaseSince < Condition # # This is eligible if the user's most recently completed order is more than the preferred days ago # @param order [Spree::Order] - def eligible?(order, _options = {}) + def order_eligible?(order, _options = {}) return false unless order.user last_order = last_completed_order(order.user) diff --git a/promotions/app/models/solidus_promotions/conditions/item_total.rb b/promotions/app/models/solidus_promotions/conditions/item_total.rb index c9d4ea6aee5..a9e7c51ceec 100644 --- a/promotions/app/models/solidus_promotions/conditions/item_total.rb +++ b/promotions/app/models/solidus_promotions/conditions/item_total.rb @@ -8,6 +8,7 @@ module Conditions # To add extra operators please override `self.operators_map` or any other helper method. # To customize the error message you can also override `ineligible_message`. class ItemTotal < Condition + # TODO: Remove in Solidus 5 include OrderLevelCondition preference :amount, :decimal, default: 100.00 @@ -28,7 +29,7 @@ def self.operator_options end end - def eligible?(order, _options = {}) + def order_eligible?(order, _options = {}) return false unless order.currency == preferred_currency unless total_for_order(order).send(operator, threshold) diff --git a/promotions/app/models/solidus_promotions/conditions/line_item_option_value.rb b/promotions/app/models/solidus_promotions/conditions/line_item_option_value.rb index c548b8b5311..a03e39e6c4b 100644 --- a/promotions/app/models/solidus_promotions/conditions/line_item_option_value.rb +++ b/promotions/app/models/solidus_promotions/conditions/line_item_option_value.rb @@ -3,11 +3,12 @@ module SolidusPromotions module Conditions class LineItemOptionValue < Condition + # TODO: Remove in Solidus 5 include LineItemLevelCondition preference :eligible_values, :hash - def eligible?(line_item, _options = {}) + def line_item_eligible?(line_item, _options = {}) pid = line_item.product.id ovids = line_item.variant.option_values.pluck(:id) diff --git a/promotions/app/models/solidus_promotions/conditions/line_item_product.rb b/promotions/app/models/solidus_promotions/conditions/line_item_product.rb index 6ad44d3e606..01de2ff8e3f 100644 --- a/promotions/app/models/solidus_promotions/conditions/line_item_product.rb +++ b/promotions/app/models/solidus_promotions/conditions/line_item_product.rb @@ -4,6 +4,7 @@ module SolidusPromotions module Conditions # A condition to apply a promotion only to line items with or without a chosen product class LineItemProduct < Condition + # TODO: Remove in Solidus 5 include LineItemLevelCondition MATCH_POLICIES = %w[include exclude].freeze @@ -23,7 +24,7 @@ def preload_relations [:products] end - def eligible?(line_item, _options = {}) + def line_item_eligible?(line_item, _options = {}) order_includes_product = product_ids.include?(line_item.variant.product_id) success = inverse? ? !order_includes_product : order_includes_product diff --git a/promotions/app/models/solidus_promotions/conditions/line_item_taxon.rb b/promotions/app/models/solidus_promotions/conditions/line_item_taxon.rb index 3956512060f..108dd2b9f37 100644 --- a/promotions/app/models/solidus_promotions/conditions/line_item_taxon.rb +++ b/promotions/app/models/solidus_promotions/conditions/line_item_taxon.rb @@ -3,6 +3,7 @@ module SolidusPromotions module Conditions class LineItemTaxon < Condition + # TODO: Remove in Solidus 5 include LineItemLevelCondition has_many :condition_taxons, @@ -22,7 +23,7 @@ def preload_relations [:taxons] end - def eligible?(line_item, _options = {}) + def line_item_eligible?(line_item, _options = {}) found = Spree::Classification.where( product_id: line_item.variant.product_id, taxon_id: condition_taxon_ids_with_children diff --git a/promotions/app/models/solidus_promotions/conditions/minimum_quantity.rb b/promotions/app/models/solidus_promotions/conditions/minimum_quantity.rb index 06a3ddcc694..208d8fd3e25 100644 --- a/promotions/app/models/solidus_promotions/conditions/minimum_quantity.rb +++ b/promotions/app/models/solidus_promotions/conditions/minimum_quantity.rb @@ -10,6 +10,7 @@ module Conditions # it to a simple quantity check across the entire order which would be # better served by an item total condition. class MinimumQuantity < Condition + # TODO: Remove in Solidus 5 include OrderLevelCondition validates :preferred_minimum_quantity, numericality: { only_integer: true, greater_than: 0 } @@ -26,7 +27,7 @@ class MinimumQuantity < Condition # # @param order [Spree::Order] the order we want to check eligibility on # @return [Boolean] true if promotion is eligible, false otherwise - def eligible?(order) + def order_eligible?(order, _options = {}) if benefit.applicable_line_items(order).sum(&:quantity) < preferred_minimum_quantity eligibility_errors.add( :base, diff --git a/promotions/app/models/solidus_promotions/conditions/nth_order.rb b/promotions/app/models/solidus_promotions/conditions/nth_order.rb index de4f8927343..dc6b595af69 100644 --- a/promotions/app/models/solidus_promotions/conditions/nth_order.rb +++ b/promotions/app/models/solidus_promotions/conditions/nth_order.rb @@ -3,6 +3,7 @@ module SolidusPromotions module Conditions class NthOrder < Condition + # TODO: Remove in Solidus 5 include OrderLevelCondition preference :nth_order, :integer, default: 2 @@ -14,7 +15,7 @@ class NthOrder < Condition # # Use the first order condition if you want a promotion to be applied to the first order for a user. # @param order [Spree::Order] - def eligible?(order, _options = {}) + def order_eligible?(order, _options = {}) return false unless order.user nth_order?(order) diff --git a/promotions/app/models/solidus_promotions/conditions/one_use_per_user.rb b/promotions/app/models/solidus_promotions/conditions/one_use_per_user.rb index 43a78b30162..a1cbd661fc6 100644 --- a/promotions/app/models/solidus_promotions/conditions/one_use_per_user.rb +++ b/promotions/app/models/solidus_promotions/conditions/one_use_per_user.rb @@ -3,9 +3,10 @@ module SolidusPromotions module Conditions class OneUsePerUser < Condition + # TODO: Remove in Solidus 5 include OrderLevelCondition - def eligible?(order, _options = {}) + def order_eligible?(order, _options = {}) if order.user.present? if promotion.used_by?(order.user, [order]) eligibility_errors.add( diff --git a/promotions/app/models/solidus_promotions/conditions/option_value.rb b/promotions/app/models/solidus_promotions/conditions/option_value.rb index 085cbfaa556..52b4c1bd19e 100644 --- a/promotions/app/models/solidus_promotions/conditions/option_value.rb +++ b/promotions/app/models/solidus_promotions/conditions/option_value.rb @@ -7,11 +7,11 @@ class OptionValue < Condition preference :eligible_values, :hash - def order_eligible?(order) + def order_eligible?(order, _options = {}) order.line_items.any? { |item| line_item_eligible?(item) } end - def line_item_eligible?(line_item) + def line_item_eligible?(line_item, _options = {}) LineItemOptionValue.new(preferred_eligible_values: preferred_eligible_values).eligible?(line_item) end diff --git a/promotions/app/models/solidus_promotions/conditions/product.rb b/promotions/app/models/solidus_promotions/conditions/product.rb index ac796cd972b..9879c3b916d 100644 --- a/promotions/app/models/solidus_promotions/conditions/product.rb +++ b/promotions/app/models/solidus_promotions/conditions/product.rb @@ -31,7 +31,7 @@ def eligible_products products end - def order_eligible?(order) + def order_eligible?(order, _options = {}) return true if eligible_products.empty? case preferred_match_policy diff --git a/promotions/app/models/solidus_promotions/conditions/shipping_method.rb b/promotions/app/models/solidus_promotions/conditions/shipping_method.rb index 08430d64ae4..a924a4a33c1 100644 --- a/promotions/app/models/solidus_promotions/conditions/shipping_method.rb +++ b/promotions/app/models/solidus_promotions/conditions/shipping_method.rb @@ -3,17 +3,15 @@ module SolidusPromotions module Conditions class ShippingMethod < Condition + # TODO: Remove in Solidus 5 include ShipmentLevelCondition preference :shipping_method_ids, type: :array, default: [] - def applicable?(promotable) - promotable.is_a?(Spree::Shipment) || promotable.is_a?(Spree::ShippingRate) - end - - def eligible?(promotable) + def shipment_eligible?(promotable, _options = {}) promotable.shipping_method&.id&.in?(preferred_shipping_method_ids.map(&:to_i)) end + alias_method :shipping_rate_eligible?, :shipment_eligible? end end end diff --git a/promotions/app/models/solidus_promotions/conditions/store.rb b/promotions/app/models/solidus_promotions/conditions/store.rb index 8dfbc9309ed..bd6fd3fd795 100644 --- a/promotions/app/models/solidus_promotions/conditions/store.rb +++ b/promotions/app/models/solidus_promotions/conditions/store.rb @@ -3,6 +3,7 @@ module SolidusPromotions module Conditions class Store < Condition + # TODO: Remove in Solidus 5 include OrderLevelCondition has_many :condition_stores, @@ -16,7 +17,7 @@ def preload_relations [:stores] end - def eligible?(order, _options = {}) + def order_eligible?(order, _options = {}) stores.none? || stores.include?(order.store) end diff --git a/promotions/app/models/solidus_promotions/conditions/taxon.rb b/promotions/app/models/solidus_promotions/conditions/taxon.rb index 4f3fb4690a6..de1e4654590 100644 --- a/promotions/app/models/solidus_promotions/conditions/taxon.rb +++ b/promotions/app/models/solidus_promotions/conditions/taxon.rb @@ -22,7 +22,7 @@ def preload_relations preference :match_policy, :string, default: MATCH_POLICIES.first - def order_eligible?(order) + def order_eligible?(order, _options = {}) order_taxons = taxons_in_order(order) case preferred_match_policy @@ -57,7 +57,7 @@ def order_eligible?(order) eligibility_errors.empty? end - def line_item_eligible?(line_item) + def line_item_eligible?(line_item, _options = {}) # The order level eligibility check happens first, and if none of the taxons # are in the order, then no line items should be available to check. raise "This should not happen" if preferred_match_policy == "none" diff --git a/promotions/app/models/solidus_promotions/conditions/user.rb b/promotions/app/models/solidus_promotions/conditions/user.rb index c9b26bbd61f..6b4cc870899 100644 --- a/promotions/app/models/solidus_promotions/conditions/user.rb +++ b/promotions/app/models/solidus_promotions/conditions/user.rb @@ -3,6 +3,7 @@ module SolidusPromotions module Conditions class User < Condition + # TODO: Remove in Solidus 5 include OrderLevelCondition has_many :condition_users, @@ -16,7 +17,7 @@ def preload_relations [:users] end - def eligible?(order, _options = {}) + def order_eligible?(order, _options = {}) users.include?(order.user) end diff --git a/promotions/app/models/solidus_promotions/conditions/user_logged_in.rb b/promotions/app/models/solidus_promotions/conditions/user_logged_in.rb index 45db92487cc..591487259db 100644 --- a/promotions/app/models/solidus_promotions/conditions/user_logged_in.rb +++ b/promotions/app/models/solidus_promotions/conditions/user_logged_in.rb @@ -3,9 +3,7 @@ module SolidusPromotions module Conditions class UserLoggedIn < Condition - include OrderLevelCondition - - def eligible?(order, _options = {}) + def order_eligible?(order, _options = {}) if order.user.blank? eligibility_errors.add(:base, eligibility_error_message(:no_user_specified), error_code: :no_user_specified) end diff --git a/promotions/app/models/solidus_promotions/conditions/user_role.rb b/promotions/app/models/solidus_promotions/conditions/user_role.rb index d4c816d0429..174e75ef0e2 100644 --- a/promotions/app/models/solidus_promotions/conditions/user_role.rb +++ b/promotions/app/models/solidus_promotions/conditions/user_role.rb @@ -3,14 +3,12 @@ module SolidusPromotions module Conditions class UserRole < Condition - include OrderLevelCondition - preference :role_ids, :array, default: [] MATCH_POLICIES = %w[any all].freeze preference :match_policy, default: MATCH_POLICIES.first - def eligible?(order, _options = {}) + def order_eligible?(order, _options = {}) return false unless order.user if all_match_policy? diff --git a/promotions/spec/models/concerns/conditions/line_item_level_condition_spec.rb b/promotions/spec/models/concerns/conditions/line_item_level_condition_spec.rb new file mode 100644 index 00000000000..645afcee3cf --- /dev/null +++ b/promotions/spec/models/concerns/conditions/line_item_level_condition_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe SolidusPromotions::Conditions::LineItemLevelCondition do + let(:legacy_condition) do + Class.new(SolidusPromotions::Condition) do + include SolidusPromotions::Conditions::LineItemLevelCondition + + def eligible?(_line_item) + true + end + end + end + + it "emits a warning telling the user to rename eligible? to line_item_eligible?" do + expect(Spree.deprecator).to receive(:warn).with <<~MSG + Defining `eligible?` on a promotion along with including the `LineItemLevelCondition` module is deprecated. + Rename `eligible?` to `line_item_eligible?` and stop including the `LineItemLevelCondition` module. + MSG + legacy_condition + end + + it "responds to eligible?", :silence_deprecations do + expect(legacy_condition.new.eligible?(Spree::LineItem.new)).to be true + end + + describe "#applicable?", :silence_deprecations do + it "is applicable for line items" do + expect(legacy_condition.new.applicable?(Spree::LineItem.new)).to be true + end + + it "is not applicable for orders" do + expect(legacy_condition.new.applicable?(Spree::Order.new)).to be false + end + end +end diff --git a/promotions/spec/models/concerns/conditions/order_level_condition_spec.rb b/promotions/spec/models/concerns/conditions/order_level_condition_spec.rb new file mode 100644 index 00000000000..a76450c0f88 --- /dev/null +++ b/promotions/spec/models/concerns/conditions/order_level_condition_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe SolidusPromotions::Conditions::OrderLevelCondition do + let(:legacy_condition) do + Class.new(SolidusPromotions::Condition) do + include SolidusPromotions::Conditions::OrderLevelCondition + + def eligible?(_line_item) + true + end + end + end + + it "emits a warning telling the user to rename eligible? to line_item_eligible?" do + expect(Spree.deprecator).to receive(:warn).with <<~MSG + Defining `eligible?` on a promotion along with including the `OrderLevelCondition` module is deprecated. + Rename `eligible?` to `order_eligible?` and stop including the `OrderLevelCondition` module. + MSG + legacy_condition + end + + it "responds to eligible?", :silence_deprecations do + expect(legacy_condition.new.eligible?(Spree::Order.new)).to be true + end + + describe "#applicable?", :silence_deprecations do + it "is applicable for orders" do + expect(legacy_condition.new.applicable?(Spree::Order.new)).to be true + end + + it "is not applicable for line items" do + expect(legacy_condition.new.applicable?(Spree::LineItem.new)).to be false + end + end +end diff --git a/promotions/spec/models/concerns/conditions/shipment_level_condition_spec.rb b/promotions/spec/models/concerns/conditions/shipment_level_condition_spec.rb new file mode 100644 index 00000000000..2cb336bd005 --- /dev/null +++ b/promotions/spec/models/concerns/conditions/shipment_level_condition_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe SolidusPromotions::Conditions::ShipmentLevelCondition do + let(:legacy_condition) do + Class.new(SolidusPromotions::Condition) do + include SolidusPromotions::Conditions::ShipmentLevelCondition + + def eligible?(_line_item) + true + end + end + end + + it "emits a warning telling the user to rename eligible? to line_item_eligible?" do + expect(Spree.deprecator).to receive(:warn).with <<~MSG + Defining `eligible?` on a promotion along with including the `ShipmentLevelCondition` module is deprecated. + Rename `eligible?` to `shipment_eligible?` and stop including the `ShipmentLevelCondition` module. + MSG + legacy_condition + end + + it "responds to eligible?", :silence_deprecations do + expect(legacy_condition.new.eligible?(Spree::Shipment.new)).to be true + end + + describe "#applicable?", :silence_deprecations do + it "is applicable for shipments" do + expect(legacy_condition.new.applicable?(Spree::Shipment.new)).to be true + end + + it "is not applicable for line items" do + expect(legacy_condition.new.applicable?(Spree::LineItem.new)).to be false + end + end +end diff --git a/promotions/spec/models/solidus_promotions/condition_spec.rb b/promotions/spec/models/solidus_promotions/condition_spec.rb index d600c8bb244..b5a835b3520 100644 --- a/promotions/spec/models/solidus_promotions/condition_spec.rb +++ b/promotions/spec/models/solidus_promotions/condition_spec.rb @@ -11,7 +11,7 @@ def self.model_name ActiveModel::Name.new(self, nil, "test_condition") end - def eligible?(_promotable, _options = {}) + def order_eligible?(_order, _options = {}) true end @@ -40,21 +40,138 @@ def level it { is_expected.to be_empty } end - it "forces developer to implement eligible? method" do - expect { bad_test_condition_class.new.eligible?("promotable") }.to raise_error NotImplementedError - expect { test_condition_class.new.eligible?("promotable") }.not_to raise_error - end + describe "#eligible?" do + let(:order_condition) do + Class.new(described_class) do + def order_eligible?(_order) = true + end + end + let(:line_item_condition) do + Class.new(described_class) do + def line_item_eligible?(_order) = true + end + end - it "forces developer to implement #applicable?" do - expect { bad_test_condition_class.new.applicable?("promotable") }.to raise_error NotImplementedError - expect { test_condition_class.new.applicable?("promotable") }.not_to raise_error - end + subject { condition.new.eligible?(promotable) } + + context "promotable is order" do + let(:promotable) { Spree::Order.new } + + context "if condition implements order_eligible?" do + let(:condition) { order_condition } + + it { is_expected.to be true } + end + + context "if condition does not implement order_eligible?" do + context "if condition implements order_eligible?" do + let(:condition) { line_item_condition } + + it "raises NotImplementedError" do + expect { subject }.to raise_error(NotImplementedError) + end + end + end + end + + describe "passing on options to a condition" do + let(:price_condition) do + Class.new(described_class) do + def price_eligible?(_price, options = {}) + options[:order].present? && options[:quantity] > 1 + end + end + end + + let(:order) { Spree::Order.new } + let(:price) { Spree::Price.new } + + subject { price_condition.new.eligible?(price, order:, quantity:) } + + context "with quantity 1" do + let(:quantity) { 1 } - it "forces developer to implement #level", :silence_deprecations do - expect { bad_test_condition_class.new.level }.to raise_error NotImplementedError - expect { test_condition_class.new.level }.not_to raise_error + it { is_expected.to be false } + end + + context "with quantity 2" do + let(:quantity) { 2 } + + it { is_expected.to be true } + end + + context "if condition does not care about order" do + let(:price_condition) do + Class.new(described_class) do + def price_eligible?(_price, options = {}) + options[:quantity] > 1 + end + end + end + + let(:quantity) { 2 } + + it { is_expected.to be true } + end + end end + describe "inherited hook" do + context "for a well-formed condition" do + subject(:condition) do + Class.new(described_class) do + def line_item_eligible?(_line_item, _options = {}) + true + end + end + end + + it "does not emit a deprecation warning" do + expect(Spree.deprecator).not_to receive(:warn) + condition + end + end + + context "for a legacy condition" do + subject(:condition) do + Class.new(described_class) do + def self.name + "LegacyCondition" + end + + def eligible?(_line_item, _options = {}) + true + end + end + end + + it "emits a deprecation warning" do + expect(Spree.deprecator).to receive(:warn).with(<<~MSG) + Please refactor `LegacyCondition`. You're defining `eligible?`. Instead, define method for each type of promotable + that your condition can be applied to. For example: + ``` + class MyCondition < SolidusPromotions::Condition + def applicable?(promotable) + promotable.is_a?(Spree::Order) + end + + def eligible?(order) + order.total > 20 + end + ``` + can now become + ``` + class MyCondition < SolidusPromotions::Condition + def order_eligible?(order) + order.total > 20 + end + end + ``` + MSG + condition + end + end + end it "validates unique conditions for a promotion benefit" do # Because of Rails' STI, we can't use the anonymous class here promotion = create(:solidus_promotion, :with_adjustable_benefit) diff --git a/promotions/spec/models/solidus_promotions/conditions/discounted_item_total_spec.rb b/promotions/spec/models/solidus_promotions/conditions/discounted_item_total_spec.rb index 7746a52b9ce..2fc3fa81953 100644 --- a/promotions/spec/models/solidus_promotions/conditions/discounted_item_total_spec.rb +++ b/promotions/spec/models/solidus_promotions/conditions/discounted_item_total_spec.rb @@ -9,10 +9,14 @@ preferred_operator: preferred_operator ) end - let(:order) { instance_double("Spree::Order", discountable_item_total: item_total, currency: order_currency) } + let(:order) { Spree::Order.new(currency: order_currency) } let(:preferred_amount) { 50 } let(:order_currency) { "USD" } let(:preferred_operator) { "gt" } + let(:item_total) { 0 } + before do + allow(order).to receive(:discountable_item_total).and_return(item_total) + end context "preferred operator set to gt" do context "item total is greater than preferred amount" do diff --git a/promotions/spec/models/solidus_promotions/conditions/item_total_spec.rb b/promotions/spec/models/solidus_promotions/conditions/item_total_spec.rb index ab4c3a6217e..16fc1465f8e 100644 --- a/promotions/spec/models/solidus_promotions/conditions/item_total_spec.rb +++ b/promotions/spec/models/solidus_promotions/conditions/item_total_spec.rb @@ -9,7 +9,7 @@ preferred_operator: preferred_operator ) end - let(:order) { instance_double("Spree::Order", item_total: item_total, currency: order_currency) } + let(:order) { Spree::Order.new(item_total: item_total, currency: order_currency) } let(:preferred_amount) { 50 } let(:order_currency) { "USD" } diff --git a/promotions/spec/models/solidus_promotions/conditions/one_use_per_user_spec.rb b/promotions/spec/models/solidus_promotions/conditions/one_use_per_user_spec.rb index 20fe909ff4a..798b5d56882 100644 --- a/promotions/spec/models/solidus_promotions/conditions/one_use_per_user_spec.rb +++ b/promotions/spec/models/solidus_promotions/conditions/one_use_per_user_spec.rb @@ -8,8 +8,8 @@ describe "#eligible?(order)" do subject { condition.eligible?(order) } - let(:order) { instance_double("Spree::Order", user: user) } - let(:user) { instance_double("Spree::LegacyUser") } + let(:order) { Spree::Order.new(user:) } + let(:user) { Spree::LegacyUser.new } let(:promotion) { stub_model SolidusPromotions::Promotion, used_by?: used_by } let(:used_by) { false }