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

Fixed currency convert exception #790

Merged
merged 4 commits into from
Aug 2, 2019
Merged

Fixed currency convert exception #790

merged 4 commits into from
Aug 2, 2019

Conversation

rafdol
Copy link
Contributor

@rafdol rafdol commented Jul 5, 2019

Parameter $toCurrency can be null or string or Mage_Directory_Model_Currency object. If $toCurrency is string and rate doen't exists then we have fatal error (Call to a member function ... on string against exception. This is fix for described problem.

@colinmollenhour
Copy link
Member

Seems this could have been fixed with impact to only one line of code using something like:

$toCurrency instanceof Mage_Directory_Model_Currency ? $toCurrency->getCode() : $toCurrency

@sreichel
Copy link
Contributor

sreichel commented Jul 5, 2019

Nice, this was on my to-do list, too.

@mcjwsk
Copy link

mcjwsk commented Jul 11, 2019

@colinmollenhour class names are pretty long in Magento 1, so I am not sure your suggestion is easier to read. Especially if a method call (getter) occurs.

@colinmollenhour
Copy link
Member

@colinmollenhour class names are pretty long in Magento 1, so I am not sure your suggestion is easier to read. Especially if a method call (getter) occurs.

Sure, but the main motivation is +1,-1 lines changed instead of +13,-6. Fewer lines changed, less chance for regression or future conflict.

@rafdol
Copy link
Contributor Author

rafdol commented Jul 19, 2019

Changed. Now it is one line change.

Copy link
Member

@colinmollenhour colinmollenhour left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@colinmollenhour colinmollenhour merged commit 72b199d into OpenMage:1.9.4.x Aug 2, 2019
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 22, 2019
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Oct 25, 2019
@sreichel sreichel added bug Component: Directory Relates to Mage_Directory labels Jun 11, 2020
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Component: Directory Relates to Mage_Directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants