Skip to content

Conversation

@elidrissidev
Copy link
Member

@elidrissidev elidrissidev commented Jan 27, 2023

Description (*)

PR created as a replacement for #2042 (comment)

When any collection is loaded, all its items have the hasDataChanges flag set to true because of the call to setData(). This causes unnecessary saves for unchanged items.

I also added a getter method to the status history model to prevent the setter from marking the model as dirty again, similar to what was done in #2066.

Related Pull Requests

Manual testing scenarios (*)

  1. Load any collection and call hasDataChanges() on the items, it should be false.
  2. Load status history collection from Order and call hasDataChanges() on the items, it should return false.
$order = Mage::getModel('sales/order')->load(123);
foreach ($order->getStatusHistoryCollection() as $history) {
    var_dump($history->hasDataChanges()); // bool(false)
}

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

@elidrissidev elidrissidev added the performance Performance related label Jan 27, 2023
@github-actions github-actions bot added Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: Sales Relates to Mage_Sales labels Jan 27, 2023
Copy link
Contributor

@kiatng kiatng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested.

@elidrissidev elidrissidev merged commit 15c1aab into OpenMage:1.9.4.x Jan 30, 2023
@elidrissidev elidrissidev deleted the fix/collection-dirty-items branch January 30, 2023 08:57
fballiano pushed a commit that referenced this pull request Jan 30, 2023
* Clear hasDataChanges flag after loading collection items

* Prevent unnecessary save of order history collection items
@fballiano
Copy link
Contributor

cherrypicked to v20

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

Labels

Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: Sales Relates to Mage_Sales performance Performance related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants