16

I found a weird bug in Magento EE 1.14.2 (also affects CE 1.9.2) with the cart.

Steps to reproduce:

  1. Log in as customer A
  2. Add product X to the cart
  3. Switch to a different browser
  4. Add product X to the cart
  5. Log in as customer A

Expected cart:

  • 2 x Product X

Actual cart:

  • 1 x Product X
  • 1 x Product X

I.e. the products are not merged.

Instead of switching the browser you could also clear the session cookie or choose a different qty for the product.

The worst side effect of this is that maximum order quantity is applied per item. In my case, there was a 100% discount on a product but you could only order it once. With this little trick, you could order it in any quantity, for free.

Why does this happen and how can I prevent it?

18

Nice wrap up of the bug above, Fabian!

For any further users who will come accross this bug, there is already a patch from Magento for this.

As an Enterprise customer, you can request/download PATCH_SUPEE-6190_EE_1.14.2.0_v1.sh to fix this.

Update 24.02.2016: This was also addressed in the latest SUPEE-7405 v 1.1 patch. According to Fabian on Twitter (see this and following tweets) there is a chance it's still not resolved completely. Please test it yourself also.

As for EE 1.14.2.0 the solution is:

diff --git a/app/code/core/Mage/Sales/Model/Quote/Item.php b/app/code/core/Mage/Sales/Model/Quote/Item.php
index 3554faa..d759249 100644
--- a/app/code/core/Mage/Sales/Model/Quote/Item.php
+++ b/app/code/core/Mage/Sales/Model/Quote/Item.php
@@ -502,8 +502,8 @@ class Mage_Sales_Model_Quote_Item extends Mage_Sales_Model_Quote_Item_Abstract
                         $itemOptionValue = $_itemOptionValue;
                         $optionValue = $_optionValue;
                         // looks like it does not break bundle selection qty
-                        unset($itemOptionValue['qty'], $itemOptionValue['uenc']);
-                        unset($optionValue['qty'], $optionValue['uenc']);
+                        unset($itemOptionValue['qty'], $itemOptionValue['uenc'], $itemOptionValue['form_key']);
+                        unset($optionValue['qty'], $optionValue['uenc'], $optionValue['form_key']);
                     }
                 }

Note: Usually I would not post EE code here, but as the problem/files are the same as in CE and do not affect a EE-only feature, I hope it's ok.

  • 4
    I approve of this. – philwinkle Jan 19 '16 at 17:15
  • 5
    We'll let it slide. – benmarks Jan 19 '16 at 18:04
  • 1
    Slide it is then. – Marius Jan 20 '16 at 8:09
  • This works better than my fix, which caused trouble with bundle products. Thanks for sharing! – Fabian Schmengler Jan 25 '16 at 11:09
  • 1
    Unfortunately this still can be bypassed if you add the product once via product list and once via product detail page, because the "related_products" parameter is only present in the latter case. You could add "related_products" to the unset() calls as well, but it's still not safe because any arbitrary POST parameter is added to the buyRequest option as well. I'm going to ignore this option completely instead. – Fabian Schmengler Feb 12 '16 at 7:57
15

It turned out that this is a bug in Mage_Sales_Model_Quote_Item::compare() that was introduced in Magento CE 1.9.2 / EE 1.14.2. The method is used to compare items to decide if they are the same product and can be merged (during login and when adding products to the cart).

When comparing all custom options, it should skip the options that are not represantative (_notRepresentOptions), namely the info_buyRequest option.

In previous Magento versions, it looked like this:

foreach ($this->getOptions() as $option) {
    if (in_array($option->getCode(), $this->_notRepresentOptions)) {
        continue;
    }

and worked correctly. Now it looks like this:

foreach ($this->getOptions() as $option) {
    if (in_array($option->getCode(), $this->_notRepresentOptions)
        && !$item->getProduct()->hasCustomOptions()
    ) {
        continue;
    }

and the additional check for hasCustomOptions() causes the described bug. Why? It looks like the check has been added to always keep products with custom options separate. I don't think it makes sense, at least not in the way it is implemented, but there will be some reason for it that I am not aware of.

However, $item->getProduct()->hasCustomOptions() always returns true for quote items!

This is the method:

public function hasCustomOptions()
{
    if (count($this->_customOptions)) {
        return true;
    } else {
        return false;
    }
}

But $this->_customOptions also contains the info_buyRequest option from the quote item.

For an unobstrusive solution, I tried to remove the info_buyRequest option from all products in an observer on sales_quote_merge_before, with no success.

The reason lies in Mage_Sales_Model_Quote_Item_Abstract::getProduct() where the option is copied again from the quote item itself:

public function getProduct()
{
    $product = $this->_getData('product');

    [...]

    if (is_array($this->_optionsByCode)) {
        $product->setCustomOptions($this->_optionsByCode);
    }
    return $product;
}

Solution

I created a rewrite for Mage_Sales_Model_Quote_Item with an override for getProduct() to not include the info_buyRequest option at this point:

public function getProduct() { $product = parent::getProduct(); $options = $product->getCustomOptions(); if (isset($options['info_buyRequest'])) { unset($options['info_buyRequest']); $product->setCustomOptions($options); } return $product; }

This caused trouble with bundle products, the alternative below or the official patch as described by @AnnaVölkl is a better solution

Alternative

You could also remove the offending && !$item->getProduct()->hasCustomOptions() in the compare() method if you are rewriting the item model anyways. I don't know what problem it tried to solve, but it created more...

Update Jan 29 2016

I reported this to Magento and got the response that they could not reproduce the issue, so the patch will not make it into the community edition (Submission APPSEC-1321).

This means, if you have the problem, you need to apply the enterprise patch SUPEE-6190 after each update or use a class rewrite instead.

  • However, $item->getProduct()->hasCustomOptions() always returns true for quote items! It is checking the product data for custom options not the quote item :) – kanevbgbe Apr 12 '18 at 9:14
  • 1
    @kanevbgbe surprisingly, no. Magento "prepares" the product instance associated with a quote item and adds its custom option values – Fabian Schmengler Apr 12 '18 at 10:36
  • I know that on add to cart action the product instance is fully loaded (compared to the quote load), so it is set from outside the quote's algorithms directly to the quote item instance trough setProduct(), maybe then this check has a different output. – kanevbgbe Apr 13 '18 at 13:25
1

As I can see the above answer is already available in latest version of Magento but we were still getting the issue. It didn't worked because we have done a lot of customizations. Thought of sharing the solution.

For us it was very simple as we use only simple products. So, we extended the quote merge compare function to this:

NS_Module_Model_Sales_Quote_Item extends Mage_Sales_Model_Quote_Item {

public function compare($item) {
    if ($this->getProductId() == $item->getProductId()) {
        return true;
    }
    return parent::compare($item);
}

}

and added

<models>
   <sales>
      <rewrite>
         <quote_item>NS_Module_Model_Sales_Quote_Item</quote_item>
      </rewrite>
   </sales>
</models>

but. for those who are using configurable products as well then it might not be helpful to you. In that case you can print both the arrays: $itemOptionValue and $optionValue and see the difference. unset all additional keys which are not common in both arrays. That should solve the problem.

-1

You can just add an option to the product in the event sales_quote_add_item:

$data['microtime'] = microtime(true);
$product->addCustomOption('do_not_merge', serialize($data));
$item->addOption($product->getCustomOption('do_not_merge'));

Reference link: Disable merging of cart positions?

  • That's a workaround, but completely disabling item merging usually is not desirable. – Fabian Schmengler Oct 2 '17 at 6:05

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.