Skip to content

Commit

Permalink
Deprecate Promotion#line_item_actionable and PromotionRule#actionable?
Browse files Browse the repository at this point in the history
  • Loading branch information
mamhoff committed Apr 1, 2022
1 parent e0b6781 commit b3d19a8
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 35 deletions.
8 changes: 4 additions & 4 deletions core/app/models/spree/calculator/distributed_amount.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ class Calculator::DistributedAmount < Calculator
def compute_line_item(line_item)
return 0 unless line_item
return 0 unless preferred_currency.casecmp(line_item.currency).zero?
return 0 unless calculable.promotion.line_item_actionable?(line_item.order, line_item)
return 0 unless calculable.promotion.line_item_eligible?(line_item)
Spree::DistributedAmountsHandler.new(
actionable_line_items(line_item.order),
eligible_line_items(line_item.order),
preferred_amount
).amount(line_item)
end

private

def actionable_line_items(order)
def eligible_line_items(order)
order.line_items.select do |line_item|
calculable.promotion.line_item_actionable?(order, line_item)
calculable.promotion.line_item_eligible?(line_item)
end
end
end
Expand Down
47 changes: 31 additions & 16 deletions core/app/models/spree/promotion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,24 +198,14 @@ def usage_count(excluded_orders: [])
end

def line_item_actionable?(order, line_item, promotion_code: nil)
return false unless line_item_eligible?(line_item)

if eligible?(order, promotion_code: promotion_code)
rules = eligible_rules(order)
if rules.blank?
true
else
rules.send(match_all? ? :all? : :any?) do |rule|
rule.actionable? line_item
end
end
else
false
end
line_item_eligible?(line_item, promotion_code: promotion_code)
end
deprecate line_item_actionable?: :line_item_eligible?

def line_item_eligible?(line_item)
!blacklisted?(line_item) && !!eligible_rules(line_item)
def line_item_eligible?(line_item, promotion_code: nil)
!blacklisted?(line_item) &&
!!eligible_rules(line_item) &&
deprecated_line_item_actionable?(line_item, promotion_code: promotion_code)
end

def used_by?(user, excluded_orders = [])
Expand Down Expand Up @@ -252,6 +242,31 @@ def remove_from(order)

private

def deprecated_line_item_actionable?(line_item, promotion_code: {})
if eligible?(line_item.order, promotion_code: promotion_code)
rules = eligible_rules(line_item.order)
if rules.blank?
true
else
rules.send(match_all? ? :all? : :any?) do |rule|
if rule.respond_to?(:actionable?)
Spree::Deprecation.warn(
<<~WARN
The API of promotion rules has changed. Rather than specifying "actionable?" on your rule, create a new rule
that is applicable to line items and move the logic in your `actionable?` method to that rule's `eligible?` method.
WARN
)
rule.actionable? line_item
else
true
end
end
end
else
false
end
end

def blacklisted?(promotable)
case promotable
when Spree::LineItem
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def perform(payload = {})
# item_total and ship_total
def compute_amount(adjustable)
order = adjustable.is_a?(Order) ? adjustable : adjustable.order
return 0 unless promotion.line_item_actionable?(order, adjustable)
return 0 unless promotion.line_item_eligible?(adjustable)
promotion_amount = calculator.compute(adjustable)
promotion_amount ||= BigDecimal(0)
promotion_amount = promotion_amount.abs
Expand Down Expand Up @@ -85,7 +85,7 @@ def ensure_action_has_calculator
def line_items_to_adjust(promotion, order)
order.line_items.select do |line_item|
line_item.adjustments.none? { |adjustment| adjustment.source == self } &&
promotion.line_item_actionable?(order, line_item)
promotion.line_item_eligible?(line_item)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def compute_amount(line_item)
adjustment_amount = adjustment_amount.abs

order = line_item.order
line_items = actionable_line_items(order)
line_items = eligible_line_items(order)

actioned_line_items = order.line_item_adjustments.reload.
select { |adjustment| adjustment.source == self && adjustment.amount < 0 }.
Expand All @@ -83,9 +83,9 @@ def compute_amount(line_item)

private

def actionable_line_items(order)
def eligible_line_items(order)
order.line_items.select do |item|
promotion.line_item_actionable? order, item
promotion.line_item_eligible? item
end
end

Expand Down
6 changes: 0 additions & 6 deletions core/app/models/spree/promotion_rule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ def eligible?(_promotable, _options = {})
raise NotImplementedError, "eligible? should be implemented in a sub-class of Spree::PromotionRule"
end

# This states if a promotion can be applied to the specified line item
# It is true by default, but can be overridden by promotion rules to provide conditions
def actionable?(_line_item)
true
end

def eligibility_errors
@eligibility_errors ||= ActiveModel::Errors.new(self)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ module Spree
context "#compute_amount" do
before { promotion.promotion_actions = [action] }

context "when the adjustable is actionable" do
context "when the adjustable is eligible" do
it "calls compute on the calculator" do
expect(action.calculator).to receive(:compute).with(line_item).and_call_original
action.compute_amount(line_item)
Expand All @@ -113,8 +113,8 @@ module Spree
end
end

context "when the adjustable is not actionable" do
before { allow(promotion).to receive(:line_item_actionable?) { false } }
context "when the adjustable is not eligible" do
before { allow(promotion).to receive(:line_ite_eligible?) { false } }

it 'returns 0' do
expect(action.compute_amount(line_item)).to eql(0)
Expand Down
8 changes: 7 additions & 1 deletion core/spec/models/spree/promotion_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,12 @@
promotion.promotion_actions = [Spree::PromotionAction.new]
end

around do |example|
Spree::Deprecation.silence do
example.run
end
end

subject { promotion.line_item_actionable? order, line_item }

context 'when the order is eligible for promotion' do
Expand Down Expand Up @@ -902,7 +908,7 @@
let(:promotion) { create(:promotion, per_code_usage_limit: 0) }
let(:promotion_code) { create(:promotion_code, promotion: promotion) }

subject { promotion.line_item_actionable? order, line_item, promotion_code: promotion_code }
subject { promotion.line_item_eligible? line_item, promotion_code: promotion_code }

it "returns false" do
expect(subject).to eq false
Expand Down

0 comments on commit b3d19a8

Please sign in to comment.