Seemingly all promotions activate on add-to-cart

[hrdchz]

When an item is added to cart, the line_item being passed to Promotion#eligible? activates all promotions. In most cases the problem is invisible: free shipping has no shipments to discount, an order adjustment will be flagged as ineligible, thought it has been placed on the order. But this behavior makes some promotions impossible, such as "if cart value exceeds $50 grant some incentive to check out"

I can say this is true of "order discount" and "free shipping". I have not had a chance to test everything.

Solidus Version:
2.10.0

To Reproduce

1. Create a promotion with a rule like "Item Total > 40.00"
2. Add a whole-order adjustment as the Action
3. From the frontend, add a product to the cart
4. Look in the spree_adjustments table
5. The order will have an ineligible adjustment, because the promo was activated, even though it was under the threshold.

Current behavior
The problem stems from perhaps a couple places. But essentially, any line item sent to the promotion model will cause a promotion to be eligible.

Expected behavior
Add-to-cart should not activate every promotion.

Additional context
Possibly tangential to #3348

The issue seems to be due to the combination of this condition in PromotionHandler::Cart#activate:

# core/app/models/spree/promotion_handler/cart.rb:27
          if (line_item && promotion.eligible?(line_item, promotion_code: promotion_code(promotion))) || promotion.eligible?(order, promotion_code: promotion_code(promotion))

and then the related logic in Promotion:

# core/app/models/spree/promotion.rb:127
    def eligible?(promotable, promotion_code: nil)
      # ...
      !!eligible_rules(promotable, {})
    end

# core/app/models/spree/promotion.rb:135
    def eligible_rules(promotable, options = {})
      return [] if rules.none?
      eligible = lambda { |r| r.eligible?(promotable, options) }
      specific_rules = rules.for(promotable)

# this return makes any promotion valid when a line item is passed in here
      return [] if specific_rules.none?
     # ... 
   end

I'm able to put a workaround in our custom promotions but basically it seems the current rule is too permissive and it does create unnecessary / invalid adjustments as it is. I would like to take the time and write all the tests / fix the root problem with a PR but today is not the day. I'm also not sure what the exact desired behavior this situation should be as I have not yet had a chance to look at how "line item" promotions are supposed to work either.

[hrdchz]

Now I am wondering about what the utility / reason for checking the rules of a line_item. I think I may be a little lost perhaps, but under what conditions would a line item have applicable rules?

Here are the results of rg promotable.is_a

guides/source/developers/promotions/promotion-rules.html.md
69:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/product.rb
27:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/item_total.rb
16:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/first_repeat_purchase_since.rb
12:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/user_role.rb
13:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/user.rb
13:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/store.rb
13:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/taxon.rb
17:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/first_order.rb
10:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/option_value.rb
12:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/one_use_per_user.rb
8:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/nth_order.rb
14:          promotable.is_a?(Spree::Order)

core/app/models/spree/promotion/rules/user_logged_in.rb
8:          promotable.is_a?(Spree::Order)

Which should get the applicable? of all promo rules. Is there some other way aside from the promotion rules for something to be considered eligible? I'm not seeing how it would be possible for a line_item to have applicable rules.

[hrdchz]

ok, so after digging a little more, it looks like in 21bf387 introduced a change to zero out the adjustment, intentionally leaving it there I suppose to avoid re-processing promotions.

However, it is still the case that if a line item is checked for eligibility on a promotion, the promotion will always be shown to be eligible, as the best I can determine, a line item will never have any applicable rules. In the case where the only bonuses an action grants are adjustments, everything works out. But for the case where you have a custom promotion that does not grant a simple adjustment, the promotion will never de-activate from the order.

[mamhoff]

Fixed with #5805