Skip to content

Conversation

@spinsch
Copy link
Contributor

@spinsch spinsch commented Sep 13, 2018

if store-id stored into model data array, the whole history collection of an order will get updated.

to reduce unnecessary mysql updates, i've used the store id directly from the order object.

fyi: all changes into a model data array will set a flag (save me again).

if store-id stored into model data array, the whole history collection of an order will get updated.

to reduce unnecessary mysql updates, i've used the store id directly from the order object.

fyi: all changes into a model data array will set a flag (save me again).
@Flyingmana
Copy link
Contributor

how does this affect order comments done from the Customer via Frontend?

@spinsch spinsch closed this Sep 27, 2018
@colinmollenhour
Copy link
Member

Good observation, I've seen this in many places where the setter causes the hasDataChanges flag to be true resulting in needless saves. The pattern I prefer to use is to check the value before setting it. In this case something like:

    public function setOrder(Mage_Sales_Model_Order $order)
    {
        $this->_order = $order;
        if ($this->getStoreId() != $order->getStoreId()) {
            $this->setStoreId($order->getStoreId());
        }
        return $this;
    }

@Flyingmana Flyingmana reopened this Sep 28, 2018
@tbaden tbaden added the review needed Problem should be verified label Jun 1, 2019
@sreichel sreichel added Component: Sales Relates to Mage_Sales enhancement performance Performance related labels Jun 5, 2020
@tmotyl
Copy link
Contributor

tmotyl commented Dec 31, 2020

Can we agree how to proceed with this one?

@kiatng
Copy link
Contributor

kiatng commented Mar 25, 2022

I tested this PR in backend order page by adding test comments, it doesn't prevent saving of the history object as intended. This is probably due to load() in the collection that calls setData():

foreach ($data as $row) {
$item = $this->getNewEmptyItem();
if ($this->getIdFieldName()) {
$item->setIdFieldName($this->getIdFieldName());
}
$item->addData($row);
$this->addItem($item);
}

I will create another PR that will fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Sales Relates to Mage_Sales enhancement performance Performance related review needed Problem should be verified

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants