creditmemo bundle inventory calculation error

[JonLaliberte]

This bug was present beginning in 1.7.0.2 and is still present in 1.9.3.3. This patch has been running in production for about 4 years without any issues.

Code was mis-calcuating the qty of simple items to put back in stock for bundle (and configurable) products.

For example if you had a Bundle X that contained 10 Widgets and the customer ordered 10 of the Bundle X (thus they ordered 100 Widgets), the code previously would multiply twice when issuing a credit memo: When calling $item->getQty() on the Widget product Magento would return 100, since that's how many exist in the order. It would then multiply that by the number of Bundle X in the order (10), and would return 1000 items to the inventory for the Widget, instead of just 100.

[colinmollenhour]

Did not test but looks good to me.

[rafaelpatro]

Hi guys
I was trying this patch.
And I have issues when refunding Configurable Products.

After applying the patch that products are not re-stocked correctly.
Steps to reproduce:

1. Apply the patch
2. Open a Configurable Product
3. Choose an associated Simple Product
4. Check the Simple Product stock qty.
5. Create an order with that item (qty 2 or more).
6. Check stock decreased by 2 in Simple Product
7. Invoice the order
8. Refund the order, selecting to back itens to stock.
9. Check stock increased by 1 in Simple Product (it must be 2)

[JonLaliberte]

@rafaelpatro It's been some time since I last looked at this code (or most any Magento code, as I'm no longer as involved in the dev side of things). Anyway, in the meantime, we've had to expand on this patch due to some other modules we've added related to inventory management.

I didn't write this, but the code below is currently running and works as expected for Bundles and Configurable items. Please take a look, and if you confirm it fixes the issues, please feel free to submit it as a new patch as I won't have time to do so.

$qty = $parentItem ? ($parentItem->getQty() * $item->getQty()) : $item->getQty();
if ($parentItem && $this->isBundle($parentItem->getProductId()) && !$this->priceDynamic($parentItem->getProductId())) {
	$bundled_product = new Mage_Catalog_Model_Product();		
	$bundled_product->load($parentItem->getProductId());
	$selectionCollection = $bundled_product->getTypeInstance(true)->getSelectionsCollection(
			$bundled_product->getTypeInstance(true)->getOptionsIds($bundled_product), $bundled_product
	);
	$bundled_items = array();
	foreach($selectionCollection as $option) {
		if ($item->getProductId() == $option->getProductId()){
			$qty = $qty * $option->getSelectionQty();
			break;
		}
	}
$item->getOrderItem()->setQtyRefunded($item->getOrderItem()->getQtyRefunded() + $qty - 1)->save();
}

if (isset($items[$item->getProductId()])) {
	$items[$item->getProductId()]['qty'] += $qty;
} else {
	$items[$item->getProductId()] = array(
		'qty' => $qty,
		'item'=> null,
	);
}

[rafaelpatro]

Hi @JonLaliberte
Thanks for your fast response.

But I can't reproduce your issue as you described.
When ordering multiple bundle with multiple single products the refund works properly.
My Mage_CatalogInventory_Model_Observer:

    public function refundOrderInventory($observer)
    {
        /* @var $creditmemo Mage_Sales_Model_Order_Creditmemo */
        $creditmemo = $observer->getEvent()->getCreditmemo();
        $items = array();
        foreach ($creditmemo->getAllItems() as $item) {
            /* @var $item Mage_Sales_Model_Order_Creditmemo_Item */
            $return = false;
            if ($item->hasBackToStock()) {
                if ($item->getBackToStock() && $item->getQty()) {
                    $return = true;
                }
            } elseif (Mage::helper('cataloginventory')->isAutoReturnEnabled()) {
                $return = true;
            }
            if ($return) {
                $parentOrderId = $item->getOrderItem()->getParentItemId();
                /* @var $parentItem Mage_Sales_Model_Order_Creditmemo_Item */
                $parentItem = $parentOrderId ? $creditmemo->getItemByOrderId($parentOrderId) : false;
                $qty = $parentItem ? ($parentItem->getQty() * $item->getQty()) : $item->getQty();
                if (isset($items[$item->getProductId()])) {
                    $items[$item->getProductId()]['qty'] += $qty;
                } else {
                    $items[$item->getProductId()] = array(
                        'qty'  => $qty,
                        'item' => null,
                    );
                }
            }
        }
        Mage::getSingleton('cataloginventory/stock')->revertProductsSale($items);
    }

[JonLaliberte]

@rafaelpatro Correct, the bundles must contain multiple items for the issue to be noticed.
From my original comment: "For example if you had a Bundle X that contained 10 Widgets and the customer ordered 10 of the Bundle X (thus they ordered 100 Widgets)."
To be clear, that means the bundle should contain 10 of "Simple Item Y" for example.

Give this a try and see if you can reproduce the original issue?

[rafaelpatro]

But this is the point. My test is Ok.

Check stock qty: 10
Order a product bundle (qty 2) with associated single (qty 3)
Qty ordered: 6
Check stock qty: 4
Invoice
Check stock qty: 4
Credit Memo (selecting back to stock)
Check stock qty: 10

So Core code works as expected. I'm not facing issues refunding bundle products.

[JonLaliberte]

@rafaelpatro
Interesting. It could be that something was changed to fix this after 1.7.0.2 that I didn't pick up, or that there is something else causing the issue.

I don't have a stock install set up to test with myself. I'm not sure if this could be affecting things, but here is how the items in our bundles are set up:

(These are "drop-down" type inputs, on the front-end we hide from the customer that it's a bundle.)

[rafaelpatro]

Hmmm my bundle product is configured by dynamic price.
I can't setup price by associated item.

[rafaelpatro]

[digitalpianism]

@JonLaliberte @rafaelpatro I made some tests too and I can confirm that I cannot reproduce the original issue Jon was having with the code from Vanilla Magento. I suggest we rollback that change as it's definitely causing issues.

[sreichel]

+1 for rollback

[addison74]

As I see there is a request for rollback of this PR. Any other opinions?