Bypass custom options checks for bundled child items

[mattdavenport]

Description (*)

We came across an alarming issue when dealing with bundled products. Seemingly randomly, some orders were getting double the child items shipped even though there was only a single parent item in the cart. After digging in, I noticed an issue inside Mage_Sales_Model_Quote_Item::representProduct. If a customer has a bundled product in their cart, then a child item's price is updated before checkout and the user adds the bundled item to their cart again, this method returns false in the compareOptions checks due to non-matching option values in sales_flat_quote_item_option. This then causes the _addCatalogProduct method to trigger adding a new quote item (per the $newItem) flag. The result is a duplicated simple product quote item entry, which is then propagated into an order and shipped unknowingly.

This isn't a frequent case, but depending on the product types and costs can be disastrous as we are effectively shipping free product without any intention or visibility.

Manual testing scenarios (*)

I made sure to replicate this on a fresh OM install to confirm this wasn't a bug in our module code. The steps are as follows:

1. Create a bundled product with 1 or more simple child products (fixed or dynamic pricing, user-inputted or fixed qtys)
2. Add the bundled product to cart
3. Change the price of the bundled product and/or child items
4. Add the bundled product to cart again

You should be able to see an additional quote item in sales_flat_quote_item after adding the product again. This however does not show up on the frontend, but will be submitted to the order quantities. You may also observe sales_flat_quote_item_option, particularly the bundle_selection_attributes code, to see the differences in values which cause compareOptions to fail.

Questions or comments

I found a comment of a similar, unaddressed issue in the M2 repo here.

It is worth noting that the bundle_selection_attributes serialized array is also used for some backend displays in admin. This particular fix implies that the prices will not be updated in these options, so when viewing an order under this scenario the user will still see item priced at the previous value before their update.

Another solution I was playing around with was, instead of simply returning true here, to instead call $this->setOptions($product->getCustomOptions());. This method actually does update the quote item options to the latest price, keeping consistent with the UI and the final amount charged to the customer. Although I did not find any regressions in tested with this solution, I figured it would be safer to not clobber existing options data at the expense of a UI inconvenience. If someone could confirm this is safe, it would be the more complete way to handle this bug. It appears for fixed priced bundled items, price is not populated in the quote_item table, so this may be a better long term fix for reporting, etc.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)
• Add yourself to contributors list

[fballiano]

is it possible that a product that's not simple gets here? I mean, are we sure that there's no other case that needs to proceed to the compareOptions part?

this is a though one

[mattdavenport]

Yeah this is indeed a tricky bug. Compare items looks at a few keys, excluding buyRequest as shown in the same file:

    /**
     * Not Represent options
     *
     * @var array
     */
    protected $_notRepresentOptions = ['info_buyRequest'];

The goal of the function is to match a newly added product with an existing item in the quote. I don't think a simple product will ever make it to this block, because at this point we've checked:

• Does the product have a parent product?
• Does the parent product match the 'child' product we're currently working with

If nothing has changed in the quote_item_options for the particular child, there is no point in comparing options anyways. However in the case where a customer re-adds an item that was existing after a product change, we may have some discrepancies. To add context here, this is an example of the option failing for the bundled product that caused this bug: a:4:{s:5:"price";d:126.5;s:3:"qty";d:2;s:12:"option_label";s:22:"CHILDSKU";s:9:"option_id";i:89;}

There isn't much metadata there, but the price alone causes compareOptions to fail, indicating that this is a new unmatched product and should be added to the quote. I'm definitely open to other suggestions here, as we admittedly aren't heavy users of bundled/configurable products. The other solution of setting the options directly the $product model's options might work, but perhaps would cause issues on child products with user-defined quantities.