Fixed PR #523, prevent unnecessary saving of status history when orde…

[kiatng]

…r is saved.

Description (*)

Order is saved when comments are added in the backend. Before this PR, this will save the new comment (status history object) along with all the old status histories which are unnecessary. After this PR, only the new comment is saved.

Related Pull Requests

PR #523

Fixed Issues (if relevant)

Manual testing scenarios (*)

To test this, add test code in app\code\core\Mage\Sales\Model\Order\Status\History.php

    public function save()
    {
        Mage::helper('clog')->log("sh#{$this->getId()}"); // use your own log
        return parent::save();
    }
   //...
    protected function _beforeSave()
    {
        Mage::helper('clog')->log("bsh#{$this->getId()}"); // use your own log
       //...

Test result before PR:

[25-03-2022 15:59:52][kiat Administrators] sh#5774133
[25-03-2022 15:59:52][kiat Administrators] bsh#5774133
[25-03-2022 15:59:52][kiat Administrators] sh#5774132
[25-03-2022 15:59:52][kiat Administrators] bsh#5774132
[25-03-2022 15:59:52][kiat Administrators] sh#5774131
[25-03-2022 15:59:52][kiat Administrators] bsh#5774131
[25-03-2022 15:59:52][kiat Administrators] sh#5773982
[25-03-2022 15:59:52][kiat Administrators] bsh#5773982
[25-03-2022 15:59:52][kiat Administrators] sh#
[25-03-2022 15:59:52][kiat Administrators] bsh#

Test result after PR:

[25-03-2022 16:00:49][kiat Administrators] sh#5774134
[25-03-2022 16:00:49][kiat Administrators] sh#5774133
[25-03-2022 16:00:49][kiat Administrators] sh#5774132
[25-03-2022 16:00:49][kiat Administrators] sh#5774131
[25-03-2022 16:00:49][kiat Administrators] sh#5773982
[25-03-2022 16:00:49][kiat Administrators] sh#
[25-03-2022 16:00:49][kiat Administrators] bsh#

Questions or comments

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]

it seems we need to add this setDataChanges calls in a lot of places, but doesn't it mean we should modify some Abstract or something like that to solve this issue in a more general way?

[kiatng]

@fballiano is right, there should be a general way to solve this, but I have not found the way to do it.

[elidrissidev]

While the current change will work fine, I found that all collections suffer from this issue! This could be fixed in all places by applying the below patch to lib/Varien/Data/Collection/Db.php, and keeping the change from the original PR to prevent setStoreId() call from changing _hasDataChanges to true again (tested).

diff --git a/lib/Varien/Data/Collection/Db.php b/lib/Varien/Data/Collection/Db.php
index 147f25ca3d..ff85dcaada 100644
--- a/lib/Varien/Data/Collection/Db.php
+++ b/lib/Varien/Data/Collection/Db.php
@@ -587,6 +587,7 @@ class Varien_Data_Collection_Db extends Varien_Data_Collection
                     $item->setIdFieldName($this->getIdFieldName());
                 }
                 $item->addData($row);
+                $item->setDataChanges(false);
                 $this->addItem($item);
             }
         }

What do you think?

[kiatng]

@elidrissidev It's a good idea. Can you create a PR and close this one?

[elidrissidev]

@kiatng Sure. Will do it as soon as I get some time.