Magento 1: Can’t Delete from a Quote Object

I’m working on an in-depth ERP project, and ran into a strange problem where I had a reference to a quote object that refused to delete its items. I could run something like this

foreach($quote->getItemsCollection() as $item)
{
    if("item meets my criteria")
    {
        $quote->deleteItem($item);
    }
}
$quote->collectTotals()->save();

but the items remained in the collection. After a few false starts down the collection caching and _hasModelChanged path I finally found the solution. From a high level? Don’t build your own ORM.

Quote Item Deleting

When you call the deleteItem method on a quote, you’re not actually deleting anything. This method will, at the end of its call-stack, simply mark the quote item for deletion.

#File: app/code/core/Mage/Sales/Model/Quote.php
$item->isDeleted(true);

Then, in the quote’s _afterSave method

#File: app/code/core/Mage/Sales/Model/Quote.php
protected function _afterSave()
{
    parent::_afterSave();
    if (null !== $this->_addresses) {
        $this->getAddressesCollection()->save();
    }

    if (null !== $this->_items) {
        $this->getItemsCollection()->save();
    }

    if (null !== $this->_payments) {
        $this->getPaymentsCollection()->save();
    }
    return $this;
}

there’s a call to the item collection’s save method

$this->getItemsCollection()->save();

Saving a collection means foreaching over each item in the collection, and calling its save method.

#File: app/code/core/Mage/Core/Model/Resource/Db/Collection/Abstract.php
public function save()
{
    foreach ($this->getItems() as $item) {
        $item->save();
    }
    return $this;
}        

Finally, in the base Mage_Core_Mode_Abstract::save method (the save method for all Magento’s CRUD objects), the first conditional takes care of deleting an item if it’s been marked with the isDeleted flag.

#File: app/code/core/Mage/Core/Model/Abstract.php    
public function save()
{
    /**
     * Direct deleted items to delete method
     */
    if ($this->isDeleted()) {
        return $this->delete();
    }
    //...
}    

A little abstract, but it makes sense once it’s been explained. After an afternoon of debugging, I’d figured out that my quote items were never being marked as deleted.

The deleteItem method eventually calls the removeItem method.

public function removeItem($itemId)
{
    $item = $this->getItemById($itemId);
    if ($item) {
        //...
    }
    //...
}    

And in my specific use case – getItemById was failing. Why? Because I’d created by object with the Mage_Sales_Model_Quote::merge method.

$quote->merge($original_quote);

The merge method copies all the quote items from one quote to another, with code that looks something like this

#File: app/code/core/Mage/Sales/Model/Quote.php
public function merge(Mage_Sales_Model_Quote $quote)
{
    //...

    foreach ($quote->getAllVisibleItems() as $item) {
        //...
            $newItem = clone $item;
            $this->addItem($newItem);
            if ($item->getHasChildren()) {
                foreach ($item->getChildren() as $child) {
                    $newChild = clone $child;
                    $newChild->setParentItem($newItem);
                    $this->addItem($newChild);
                }
            }
        //...
    }
    //...
}  

The key line here is $newChild = clone $child; – this clones the original quote item. Thanks to some code in the base sales object abstract class

#File: app/code/core/Mage/Sales/Model/Quote/Item/Abstract.php     
public function __clone()
{
    $this->setId(null);
    $this->_parentItem  = null;
    $this->_children    = array();
    $this->_messages    = array();
    return $this;
}

any cloned item will have its ID nulled out. This was what caused my problem. Remember this code?

public function removeItem($itemId)
{
    $item = $this->getItemById($itemId);
    if ($item) {
        //...
    }
    //...
}    

If we take a look at the base collection object’s getItemById method

#File: lib/Varien/Data/Collection.php
public function getItemById($idValue)
{
    $this->load();
    if (isset($this->_items[$idValue])) {
        return $this->_items[$idValue];
    }
    return null;
}

We can see it assumes an item has been added to the collection’s internal _items array by its ID value. This is normally the case – however, if an item doesn’t have an ID (as our cloned items do not), the item is just pushed on to the end of the _items array

#File: lib/Varien/Data/Collection.php
public function addItem(Varien_Object $item)
{        
    $itemId = $this->_getItemId($item);

    if (!is_null($itemId)) {
        //...
    } else {                    
        $this->_addItem($item);
    }
    return $this;
} 

#File: lib/Varien/Data/Collection.php
protected function _addItem($item)
{
    // var_dump($item);
    $this->_items[] = $item;
    return $this;
}   

This will give the PHP array in _items incrementing numerical keys, which means the item can’t be fetched via an ID – even it the item object is later given an ID.

Once I’d done this digging, I was able to solve the problem by reloading the items collection on my quote object after calling merge. My entire call looked like this, with $quote being the quote I was trying to copy.

//the entire merge call
$new_quote->merge($quote)
    ->setStoreId($quote->getStoreId())
    ->assignCustomerWithAddressChange(  
        $quote->getCustomer(), 
        $quote->getBillingAddress(), 
        $quote->getShippingAddress())
    ->merge($quote)
    ->collectTotals()
    ->save();   

//reloading the items collection so it has properly indexed `_items`                 
$new_quote->getItemsCollection()
    ->setQuote($new_quote)       //needed or Magento tries to load ALL items
    ->clear()
    ->load();

Not pretty, but the Mage_Sales code is some of the oldest and most “refactored without open source tests” code in the system. Always an adventure.

Using Native CRUD Objects?

It’s stuff like this that usually leads me away from using the base CRUD objects, and more towards using the PHP objects that implement the Magento API. The problem is – there’s no API objects that deal directly with quotes. It’s all masked via the sales and checkout APIs. The closest thing I found to do what I wanted was this

Mage::getModel('checkout/cart_product_api')
    ->remove($quoteId, $productsData, $store = null);

Which lets you remove a specific product from the cart. Going with this would have meant

• Rethinking the entire semantics of my program
• Living with a vastly less performant API call

The later made using checkout/cart_product_api a non starter.

Sometimes, despite everyone’s best efforts, programming is just work.