8

There appears to be a bug with "Stop Further Rules Processing" in Magento CE1.9 / EE1.13 where only the first item in your cart recieves the discount.

I would expect: If I have multiple shopping cart rules, each of which having "Stop Further Rules Processing: Yes", only the first of these rules would be applied, however it would be applied in full to all matching items for that rule.

What is happening: The discount is only being applied to the first item in the cart, after which the rule processing is stopped.

See screenshots: The discount I am expecting for the entire cart is $50, but due to "Stop Further Rules Processing" I am only seeing $25.

Magento Admin Panel

Magento Frontend Checkout

7

I think this could be because the _calculator is effectively stored as a singleton within the Mage_SalesRule_Model_Quote_Discount class, meaning that the second item to be processed will hit $this->_stopFurtherRules == true and bail.

My thought process is to store the ID of the $rule which is OK to be processed, allowing further items to process only this rule.

As per CE1.9.0.1 and EE1.14.0.1

Mage_SalesRule_Model_Validator line 316

- if ($this->_stopFurtherRules) {
+ if ($this->_stopFurtherRules !== false && $rule->getId() != $this->_stopFurtherRules) {

Mage_SalesRule_Model_Validator line 514

- $this->_stopFurtherRules = true;
+ $this->_stopFurtherRules = $rule->getId();

This is my proposed solution, I would be interested to hear reasons why this is a terrible idea!

2

What worked for me was resetting the stop further rules flag after each item is processed to allow the next item to check the rules against it.

add this line:

$this->_stopFurtherRules = false;

directly after this loop in the process() method:

foreach ($this->_getRules() as $rule) {
    ...
}

That was on line 518, for me.

In my opinion, Magento has it back-to-front. It iterates the items, then the rules for each item. It should be iterating the rules, then the items, so a rule can apply to the whole cart and only then prevent further discounts.

  • The problem with that approach is (and this is an assumption just from glancing at the code, ive not tested) you lose the 'stop further rules' functionality, ie. all of your rules will be processed, even if you have a rule that should only be applied on its own. Update: I agree with the back-to-front comment, I believe it should process rules then items. – Joseph McDermott Feb 19 '15 at 10:34
  • @JosephMcDermott That's not correct. It still stops further processing of rules for that item. For many discounts, it will stop at the same rule for each item. For any items that a previously matched rule does not apply to, they can only be discounted as much as other applicable rules allow. And doesn't Magento only allow a single coupon code to be used at once? – Walf Feb 20 '15 at 8:32
  • I think you've misunderstood the intention of the stop further rules flag, it's for rule level not item level. If you have two promo rules, neither of which require a promo code, the first for 30% if you spend £300, the second for 20% if you spend £200, you would mark the first rule as lower priority with stop further rules processing: yes so that the customer would only get 30% discount, instead of 30% followed by %20. Or you might have a global sale of 10% off everything (no promo) but if the customer enters a promo code you dont want the customer to get this, so use stop further rules. – Joseph McDermott Feb 21 '15 at 14:11
  • @JosephMcDermott No, I haven't. I'm well aware of the purpose of that flag, but Magento clearly does not use it as one expects. My solution allows each item to go through the rules, at least until they hit that flag. I'm sure there is a better way to prevent further rule processing completely and discount the whole cart, but I guarantee it's far more complex than resetting one variable. – Walf Feb 23 '15 at 8:52
  • OK well at least we agree that Magento have done this badly :) The public now have two solutions to choose from, either of which should at least point developers in the right direction. – Joseph McDermott Feb 24 '15 at 9:27
2

This was fixed in a later version of Magento CE. In 1.9.2.1 you can find the solution, but it may have been fixed sooner.

The original code looks like this:

$appliedRuleIds = array();
foreach ($this->_getRules() as $rule) {
    if ($this->_stopFurtherRules) {
        break;
    }

And the fixed code should be:

$appliedRuleIds = array();
$this->_stopFurtherRules = false;
foreach ($this->_getRules() as $rule) {
    // The if-clause is removed
    ...    

The differennce is the $this->_stopFurtherRules = false; and if ($this->_stopFurtherRules) {...}

Nothing else.

Or, if you are on 1.9 you can simply replace the entire file without danger.

Hope this helps someone.

1

For all that needs to fix that issue, should override process method for Mage_SalesRule_Model_Validator class to be like below

public function process(Mage_Sales_Model_Quote_Item_Abstract $item)
{
    $item->setDiscountAmount(0);
    $item->setBaseDiscountAmount(0);
    $item->setDiscountPercent(0);
    $quote      = $item->getQuote();
    $address    = $this->_getAddress($item);

    $itemPrice              = $this->_getItemPrice($item);
    $baseItemPrice          = $this->_getItemBasePrice($item);
    $itemOriginalPrice      = $this->_getItemOriginalPrice($item);
    $baseItemOriginalPrice  = $this->_getItemBaseOriginalPrice($item);

    if ($itemPrice < 0) {
        return $this;
    }

    $appliedRuleIds = array();
    $this->_stopFurtherRules = false;
    foreach ($this->_getRules() as $rule) {

        /* @var $rule Mage_SalesRule_Model_Rule */
        if (!$this->_canProcessRule($rule, $address)) {
            continue;
        }

        if (!$rule->getActions()->validate($item)) {
            continue;
        }

        $qty = $this->_getItemQty($item, $rule);
        $rulePercent = min(100, $rule->getDiscountAmount());

        $discountAmount = 0;
        $baseDiscountAmount = 0;
        //discount for original price
        $originalDiscountAmount = 0;
        $baseOriginalDiscountAmount = 0;

        switch ($rule->getSimpleAction()) {
            case Mage_SalesRule_Model_Rule::TO_PERCENT_ACTION:
                $rulePercent = max(0, 100-$rule->getDiscountAmount());
            //no break;
            case Mage_SalesRule_Model_Rule::BY_PERCENT_ACTION:
                $step = $rule->getDiscountStep();
                if ($step) {
                    $qty = floor($qty/$step)*$step;
                }
                $_rulePct = $rulePercent/100;
                $discountAmount    = ($qty * $itemPrice - $item->getDiscountAmount()) * $_rulePct;
                $baseDiscountAmount = ($qty * $baseItemPrice - $item->getBaseDiscountAmount()) * $_rulePct;
                //get discount for original price
                $originalDiscountAmount    = ($qty * $itemOriginalPrice - $item->getDiscountAmount()) * $_rulePct;
                $baseOriginalDiscountAmount =
                    ($qty * $baseItemOriginalPrice - $item->getDiscountAmount()) * $_rulePct;

                if (!$rule->getDiscountQty() || $rule->getDiscountQty()>$qty) {
                    $discountPercent = min(100, $item->getDiscountPercent()+$rulePercent);
                    $item->setDiscountPercent($discountPercent);
                }
                break;
            case Mage_SalesRule_Model_Rule::TO_FIXED_ACTION:
                $quoteAmount = $quote->getStore()->convertPrice($rule->getDiscountAmount());
                $discountAmount    = $qty * ($itemPrice-$quoteAmount);
                $baseDiscountAmount = $qty * ($baseItemPrice-$rule->getDiscountAmount());
                //get discount for original price
                $originalDiscountAmount    = $qty * ($itemOriginalPrice-$quoteAmount);
                $baseOriginalDiscountAmount = $qty * ($baseItemOriginalPrice-$rule->getDiscountAmount());
                break;

            case Mage_SalesRule_Model_Rule::BY_FIXED_ACTION:
                $step = $rule->getDiscountStep();
                if ($step) {
                    $qty = floor($qty/$step)*$step;
                }
                $quoteAmount        = $quote->getStore()->convertPrice($rule->getDiscountAmount());
                $discountAmount     = $qty * $quoteAmount;
                $baseDiscountAmount = $qty * $rule->getDiscountAmount();
                break;

            case Mage_SalesRule_Model_Rule::CART_FIXED_ACTION:
                if (empty($this->_rulesItemTotals[$rule->getId()])) {
                    Mage::throwException(Mage::helper('salesrule')->__('Item totals are not set for rule.'));
                }

                /**
                 * prevent applying whole cart discount for every shipping order, but only for first order
                 */
                if ($quote->getIsMultiShipping()) {
                    $usedForAddressId = $this->getCartFixedRuleUsedForAddress($rule->getId());
                    if ($usedForAddressId && $usedForAddressId != $address->getId()) {
                        break;
                    } else {
                        $this->setCartFixedRuleUsedForAddress($rule->getId(), $address->getId());
                    }
                }
                $cartRules = $address->getCartFixedRules();
                if (!isset($cartRules[$rule->getId()])) {
                    $cartRules[$rule->getId()] = $rule->getDiscountAmount();
                }

                if ($cartRules[$rule->getId()] > 0) {
                    if ($this->_rulesItemTotals[$rule->getId()]['items_count'] <= 1) {
                        $quoteAmount = $quote->getStore()->convertPrice($cartRules[$rule->getId()]);
                        $baseDiscountAmount = min($baseItemPrice * $qty, $cartRules[$rule->getId()]);
                    } else {
                        $discountRate = $baseItemPrice * $qty /
                            $this->_rulesItemTotals[$rule->getId()]['base_items_price'];
                        $maximumItemDiscount = $rule->getDiscountAmount() * $discountRate;
                        $quoteAmount = $quote->getStore()->convertPrice($maximumItemDiscount);

                        $baseDiscountAmount = min($baseItemPrice * $qty, $maximumItemDiscount);
                        $this->_rulesItemTotals[$rule->getId()]['items_count']--;
                    }

                    $discountAmount = min($itemPrice * $qty, $quoteAmount);
                    $discountAmount = $quote->getStore()->roundPrice($discountAmount);
                    $baseDiscountAmount = $quote->getStore()->roundPrice($baseDiscountAmount);

                    //get discount for original price
                    $originalDiscountAmount = min($itemOriginalPrice * $qty, $quoteAmount);
                    $baseOriginalDiscountAmount = $quote->getStore()->roundPrice($baseItemOriginalPrice);

                    $cartRules[$rule->getId()] -= $baseDiscountAmount;
                }
                $address->setCartFixedRules($cartRules);

                break;

            case Mage_SalesRule_Model_Rule::BUY_X_GET_Y_ACTION:
                $x = $rule->getDiscountStep();
                $y = $rule->getDiscountAmount();
                if (!$x || $y > $x) {
                    break;
                }
                $buyAndDiscountQty = $x + $y;

                $fullRuleQtyPeriod = floor($qty / $buyAndDiscountQty);
                $freeQty  = $qty - $fullRuleQtyPeriod * $buyAndDiscountQty;

                $discountQty = $fullRuleQtyPeriod * $y;
                if ($freeQty > $x) {
                    $discountQty += $freeQty - $x;
                }

                $discountAmount    = $discountQty * $itemPrice;
                $baseDiscountAmount = $discountQty * $baseItemPrice;
                //get discount for original price
                $originalDiscountAmount    = $discountQty * $itemOriginalPrice;
                $baseOriginalDiscountAmount = $discountQty * $baseItemOriginalPrice;
                break;
        }

        $result = new Varien_Object(array(
            'discount_amount'      => $discountAmount,
            'base_discount_amount' => $baseDiscountAmount,
        ));
        Mage::dispatchEvent('salesrule_validator_process', array(
            'rule'    => $rule,
            'item'    => $item,
            'address' => $address,
            'quote'   => $quote,
            'qty'     => $qty,
            'result'  => $result,
        ));

        $discountAmount = $result->getDiscountAmount();
        $baseDiscountAmount = $result->getBaseDiscountAmount();

        $percentKey = $item->getDiscountPercent();
        /**
         * Process "delta" rounding
         */
        if ($percentKey) {
            $delta      = isset($this->_roundingDeltas[$percentKey]) ? $this->_roundingDeltas[$percentKey] : 0;
            $baseDelta  = isset($this->_baseRoundingDeltas[$percentKey])
                ? $this->_baseRoundingDeltas[$percentKey]
                : 0;
            $discountAmount += $delta;
            $baseDiscountAmount += $baseDelta;

            $this->_roundingDeltas[$percentKey]     = $discountAmount -
                $quote->getStore()->roundPrice($discountAmount);
            $this->_baseRoundingDeltas[$percentKey] = $baseDiscountAmount -
                $quote->getStore()->roundPrice($baseDiscountAmount);
            $discountAmount = $quote->getStore()->roundPrice($discountAmount);
            $baseDiscountAmount = $quote->getStore()->roundPrice($baseDiscountAmount);
        } else {
            $discountAmount     = $quote->getStore()->roundPrice($discountAmount);
            $baseDiscountAmount = $quote->getStore()->roundPrice($baseDiscountAmount);
        }

        /**
         * We can't use row total here because row total not include tax
         * Discount can be applied on price included tax
         */

        $itemDiscountAmount = $item->getDiscountAmount();
        $itemBaseDiscountAmount = $item->getBaseDiscountAmount();

        $discountAmount     = min($itemDiscountAmount + $discountAmount, $itemPrice * $qty);
        $baseDiscountAmount = min($itemBaseDiscountAmount + $baseDiscountAmount, $baseItemPrice * $qty);

        $item->setDiscountAmount($discountAmount);
        $item->setBaseDiscountAmount($baseDiscountAmount);

        $item->setOriginalDiscountAmount($originalDiscountAmount);
        $item->setBaseOriginalDiscountAmount($baseOriginalDiscountAmount);

        $appliedRuleIds[$rule->getRuleId()] = $rule->getRuleId();

        $this->_maintainAddressCouponCode($address, $rule);
        $this->_addDiscountDescription($address, $rule);

        if ($rule->getStopRulesProcessing()) {
            $this->_stopFurtherRules = true;
            break;
        }
    }

    $item->setAppliedRuleIds(join(',',$appliedRuleIds));
    $address->setAppliedRuleIds($this->mergeIds($address->getAppliedRuleIds(), $appliedRuleIds));
    $quote->setAppliedRuleIds($this->mergeIds($quote->getAppliedRuleIds(), $appliedRuleIds));

    return $this;
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.