Distributed amount calculator doesn't work with product promo rule

[adammathys]

What's the problem?

Promotions using both Spree::Calculator::DistributedAmount and Spree::Promotion::Rules::Product don't correctly discount the order.

Steps to reproduce

1. Create a promotion with the following action/rule setup:
   

2. Add a matching and a non-matching product to an order and apply the promotion.

Expected behavior

We would expect the customer to receive the full amount specified in the promotion action.

Actual behavior

Customer only receives a portion of the total discount.

How's it happening?

There's a longstanding implicit interaction between Spree::Promotion::Rules::Product and Spree::Promotion::Actions::CreateItemAdjustments. Specifically, the action will only add adjustments to items that match the product(s) specified by that particular rule.

The action looks for line_items_to_adjust.

def line_items_to_adjust(promotion, order)
  excluded_ids = adjustments.
    where(adjustable_id: order.line_items.pluck(:id), adjustable_type: 'Spree::LineItem').
    pluck(:adjustable_id).
    to_set

  order.line_items.select do |line_item|
    !excluded_ids.include?(line_item.id) &&
      promotion.line_item_actionable?(order, line_item)
  end
end

If we look at Spree::Promotion#line_item_actionable?, we'll notice that it looks to the promotion rules to help determine whether the item is actionable.

def line_item_actionable?(order, line_item, promotion_code: nil)
  return false if blacklisted?(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
end

When it comes to Spree::Promotion::Rules::Product#actionable?, it only considers items matching (or not matching) it's preferred products as eligible.

def actionable?(line_item)
  case preferred_match_policy
  when 'any', 'all'
    product_ids.include? line_item.variant.product_id
  when 'none'
    product_ids.exclude? line_item.variant.product_id
  else
    raise "unexpected match policy: #{preferred_match_policy.inspect}"
  end
end

However, the distributed calculator looks at all items in the order when determining how much each item should be discounted.

How could we fix it?

I believe there are two distinct approaches we could take to fix this particular issue.

Change the calculator

We can update the distributed amount calculator to only consider "actionable" items when distributing the amount. This would require updating Spree::DistributedAmountsHandler to have knowledge of either the promotion or the set of line items it should be distributing the amount across. (I'm in favour of the latter.)

Change the action

Although I don't think we can reasonably change Spree::Promotion::Actions::CreateItemAdjustments to apply the adjustment to all items by default. We could add a way to opt out of that behaviour. Either by adding a boolean with some accompanying logic to the action or by creating a second, nearly identical action and expanding on the copy and documentation surrounding the actions.

[gevann]

I believe this is closed by #2582