Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch for code that is not following best practices in Magento development #46

Closed
wants to merge 4 commits into from

Conversation

IvanChepurnyi
Copy link
Contributor

See attached commit, where using of collection for retrieving address by id is replaced by using of standard load method of the model.

…o use more appropriate load() method of the model.
->getCollection()
->addFilter('entity_id', $addressId)
->getItemById($addressId);
$address = Mage::getModel('Mage_Sales_Model_Order_Address')->load($addressId);
if ($address) {
Copy link

Choose a reason for hiding this comment

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

Collection method getItemById returns object instance if found and null otherwise.
Your code will return Mage_Sales_Model_Order_Address instance in any case.

So you need to change if statement to be something like this:

if ($address->getId() !== null)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if ($address->getId()) is enough, just 10 lines of code later the similar construction is used.

…problem is visible if you have additional custom totals that need to be placed in between core ones.
@IvanChepurnyi
Copy link
Contributor Author

Closed in favor of #48

magento-team pushed a commit that referenced this pull request Jan 23, 2015
[Firedrakes] Bugfixes. Sprint 35
@stevieyu stevieyu mentioned this pull request Apr 3, 2015
@chrom chrom mentioned this pull request Oct 7, 2015
okorshenko pushed a commit that referenced this pull request Oct 28, 2015
magento-engcom-team pushed a commit that referenced this pull request Aug 27, 2018
magento-engcom-team pushed a commit that referenced this pull request May 30, 2021
MC-42057: Fix jQuery.fn.resize()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant