-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix cache issue for currencies with no symbol #13894
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please adopt corresponding test by fixing existing case and also add a new one, with empty currency symbol.
As always, please do amend commit and force push.
@orlangur, I fixed the test and submitted an issue with instructions on how to reproduce the problem. Regarding a separate test with an empty currency-symbol, doesn't seem like it makes sense in the context of this fix, since the whole idea is to stop using currency-symbol and use currency-code for caching. |
@evgk ok, looks like there is quite low probability of regression in this place. |
Hi @orlangur, thank you for the review. |
Using getCurrencySymbol() leads to a bug for currencies where there is no currency symbol - cache ends up being non-unique. Using currency code for caching is a more foolproof way.
Contribution checklist