"Stop Further Rules Processing" not applying to all items

Question

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.

Answer 1

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!

Answer 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.

Answer 3

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.

Answer 4

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;
}

Answer 5

Comment out this line just before
foreach ($this->_getRules() as $rule) {

315 //$this->_stopFurtherRules = false;
    foreach ($this->_getRules() as $rule) {

and add the following lines just after the beginning of the for-each

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

        // Add this if
        if($this->_stopFurtherRules) {
            break;
        }