Skip to content

Fix for Issue #13950 - Cache issue with configurable products related to currency-conversions #14017

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

Closed
wants to merge 5 commits into from

Conversation

nuzil
Copy link
Contributor

@nuzil nuzil commented Mar 8, 2018

Preconditions.

Magento website with configured multiple currencies, for some of currencies not configured currency symbol.

Problem.

Two Magento block classes used in generating cache key (in method getCacheKeyInfo()) current currency symbol. It leads to problem that these blocks has same cache for different currencies which has no symbol configured.

Solution.

Use currency code instead currency symbol in cache key generating to get unique cache of these block for different currencies.

Pavel added 2 commits March 7, 2018 19:35
…Product\Block\Product\View\Type\Configurable and Magento\CatalogWidget\Block\Product\ProductsList blocks:

used currency code instead of currency symbol in cache key info array.
@orlangur orlangur self-assigned this Mar 9, 2018
Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

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

Hi @nuzil,

ConfigurableProduct case was covered by #13894 already, could you please rewrite this PR so that it contains only one commit and only changes related to CatalogWidget?

@nuzil
Copy link
Contributor Author

nuzil commented Mar 27, 2018

Hi @orlangur
Changes are applied and pushed

@orlangur
Copy link
Contributor

Thanks @nuzil, changes look good to me now.

By "could you please rewrite this PR" I meant squashing into a single commit so that history is cleaner. Please do it and perform a force push into the same branch.

@nuzil
Copy link
Contributor Author

nuzil commented Apr 5, 2018

Sorry we have a mess already in our own repository, I made a new PR
#14559

This one can closed then

@sidolov
Copy link
Contributor

sidolov commented Apr 5, 2018

Closed according to comment above

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

Successfully merging this pull request may close these issues.

3 participants