1

I am currently going through and optimizing our codebase for our custom modules. I am using PHP Codesniffer along with the Magento Coding Standard rules to scan our code.

In many of our files I see a warning like

Model LSD method save() detected in loop

Here is an example of a function containing that error

public function resetmasstatusAction()
 {
    $invoicesIds = $this->getRequest()->getPost('invoice_ids');
    if (!empty($invoicesIds)) {
        $invoices = Mage::getResourceModel('sales/order_invoice_collection')
            ->addAttributeToSelect('*')
            ->addAttributeToFilter('entity_id', array('in' => $invoicesIds))
            ->load();
        foreach ($invoices as $invoice) :
            $invoice->setMas_status('NEW')
                ->setMas_updated_at(date('Y-m-d H:i:s'))
                ->save();
        endforeach;
    }
    $this->_redirect('*/sales_invoice/index');
}

How would I go about doing the save operation for all items in a collection such as this one, without having the save() function in the foreach loop to improve performance?

4
  • I guess no one had an answer
    – djames
    Commented Jan 30, 2017 at 17:57
  • 2
    Possible duplicate of M1 - Efficient Bulk Update Order Attribute Commented Feb 3, 2017 at 12:56
  • please check the method the OP created in his question (my answer actually includes a foreach, but the OP created his own method which does not) Commented Feb 3, 2017 at 13:00
  • @RaphaelatDigitalPianism, I actually determined how to do this and am posting my answer now.
    – djames
    Commented Feb 3, 2017 at 22:17

1 Answer 1

2

After doing some research into this, I discovered that the best way to do this is like the following.

Please note that the walk iterator is essentially the same thing as a foreach loop, however the goal in my original question was how to not run a SQL query over and over again in a foreach loop.

By running the save() function on the entire $invoices object, we essentially run a large SQL query to save all of the rows at once rather than running a bunch of small queries.

public function resetmasstatusAction()
 {
    $invoicesIds = $this->getRequest()->getPost('invoice_ids');
    if (!empty($invoicesIds)) {
        $invoices = Mage::getResourceModel('sales/order_invoice_collection')
            ->addAttributeToSelect('*')
            ->addAttributeToFilter('entity_id', array('in' => $invoicesIds))
            ->load();
        //The below will allow you to save the updated collection
        //Without running multiple SQL queries in the foreach loop.
        $invoices->walk('setMas_status', array('NEW');
        $invoices->walk('setMas_updated_at', array(date('Y-m-d H:i:s'));           
        $invoices->save();
    }
    $this->_redirect('*/sales_invoice/index');
}
1
  • 1
    The save method from the 'sales/order_invoice_collection' is defined in Mage_Core_Model_Resource_Db_Collection_Abstract, where it actually does a foreach to save each item.
    – Cladiuss
    Commented May 30, 2017 at 15:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.