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.

Sign up for GitHub

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

Jump to bottom

Fixes layered navigation options being cached using the wrong store id. #9873

Merged
merged 1 commit into from
Jun 9, 2017
Merged

Fixes layered navigation options being cached using the wrong store id. #9873

merged 1 commit into from
Jun 9, 2017

Conversation

hostep
Copy link
Contributor

@hostep hostep commented Jun 7, 2017 •
edited by okorshenko
Loading

Description

This is a "forward" port of MAGETWO-69543.
See #9704 for more details.

The reason why this was first fixed on the 2.1-develop branch, is that the feature of caching the layered navigation options only landed on the develop branch a few days after #9704 was created.

Fixed Issues (if relevant)

  1. Translation for layered navigation attribute option not working #9679: Translation for layered navigation attribute option not working

Manual testing scenarios

See #9704

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@hostep hostep mentioned this pull request Jun 7, 2017
Merged
4 tasks
@okorshenko okorshenko self-assigned this Jun 7, 2017
@okorshenko okorshenko added this to the June 2017 milestone Jun 7, 2017
@hostep hostep force-pushed the forwardport-magetwo-69543 branch from ea9d540 to 9aec90e Compare June 7, 2017 19:21
@hostep
Copy link
Contributor Author

hostep commented Jun 7, 2017 •
edited
Loading

Just pushed a new commit which should fix the unit tests.
It would be nice if someone with more knowledge about writing tests then myself could add a new test which tests the caching of options with multiple store id's, to make sure this bug doesn't resurface later on.

@magento-team magento-team merged commit 9aec90e into magento:develop Jun 9, 2017
magento-team pushed a commit that referenced this pull request Jun 9, 2017
@ishakhsuvarov
MAGETWO-69728: Fixes layered navigation options being cached using th…
7f6da71 
…e wrong store id. #9873
magento-team pushed a commit that referenced this pull request Jun 9, 2017
@ishakhsuvarov
MAGETWO-69728: Fixes layered navigation options being cached using th…
58b4c7c 
…e wrong store id. #9873

 - Backwards compatibility improvements
@hostep
Copy link
Contributor Author

hostep commented Jun 10, 2017

@magento-team or @okorshenko: why did you switch the order of the StoreManagerInterface and Serializer parameters around in the constructor? This is introducing a backwards incompatible change, if you upgrade from 2.1.x to 2.2.
Since the Serializer parameter doesn't exist in 2.1.x
2.1-develop branch vs develop branch

I did this on purpose so there would be no backwards incompatibility being introduced, but you switched it around ... The Serializer should be the last one I believe.

Thanks!

@hostep
Copy link
Contributor Author

hostep commented Jun 12, 2017

@okorshenko or @ishakhsuvarov, can you please check my previous comment? Thanks!

@okorshenko
Copy link
Contributor

Hi @hostep thank you for pointing on this. we will review the code one more time

@okorshenko
Copy link
Contributor

okorshenko commented Jun 12, 2017 •
edited
Loading

Your comment make sense. We will fix it or you can create PR and we will accept ASAP. The reason why we did that is here: http://devdocs.magento.com/guides/v2.0/contributor-guide/backward-compatible-development/#adding-a-constructor-parameter

hostep added a commit to hostep/magento2 that referenced this pull request Jun 12, 2017
@hostep
Revert "MAGETWO-69728: Fixes layered navigation options being cached …
76b45a7 
…using the wrong store id. magento#9873"

This reverts commit 58b4c7c.
@hostep hostep mentioned this pull request Jun 12, 2017
Merged
@hostep
Copy link
Contributor Author

hostep commented Jun 12, 2017

@okorshenko: here you go: #9922 :)

AntonEvers pushed a commit to AntonEvers/magento2 that referenced this pull request Jun 14, 2017
ISSUE-9924 Merge remote-tracking branch 'upstream/develop' into issue…
322c357 
…-9924

* upstream/develop: (60 commits)
  Fix typo in comment
  Move prefix and suffix default values to a new PR
  MAGETWO-68877: Issue magento#7988 Typo changed also added comments for each index, getters and setters. magento#9484
  Revert "MAGETWO-69728: Fixes layered navigation options being cached using the wrong store id. magento#9873"
  MAGETWO-67500: setup:di:compile returns exit code 0 if errors are found magento#7780
  Fix prefix, middle name and suffix were not prefilled in the checkout
  add middle name to checkout address html templates magento#8878
  Using Command output as message which actually provides more information for debugging than just the path
  Handling CLI error as a failure when validating composer.json file
  MAGETWO-69805: Return array of blocks as items instead of array of arrays magento#9157
  MAGETWO-69666: Return array of pages as items instead of array of arrays magento#9823
  MAGETWO-69723: Email to a Friend feature magento#9824
  MAGETWO-69539: PHP "soap" extension is not declared in composer.json but can be used by Magento modules
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-63054: [Catalog] MSRP field is not displayed for bundle products with fixed price
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  ...
AntonEvers pushed a commit to AntonEvers/magento2 that referenced this pull request Jun 14, 2017
Merge branch 'develop' into resizer
4dc7817 
* develop: (60 commits)
  Fix typo in comment
  Move prefix and suffix default values to a new PR
  MAGETWO-68877: Issue magento#7988 Typo changed also added comments for each index, getters and setters. magento#9484
  Revert "MAGETWO-69728: Fixes layered navigation options being cached using the wrong store id. magento#9873"
  MAGETWO-67500: setup:di:compile returns exit code 0 if errors are found magento#7780
  Fix prefix, middle name and suffix were not prefilled in the checkout
  add middle name to checkout address html templates magento#8878
  Using Command output as message which actually provides more information for debugging than just the path
  Handling CLI error as a failure when validating composer.json file
  MAGETWO-69805: Return array of blocks as items instead of array of arrays magento#9157
  MAGETWO-69666: Return array of pages as items instead of array of arrays magento#9823
  MAGETWO-69723: Email to a Friend feature magento#9824
  MAGETWO-69539: PHP "soap" extension is not declared in composer.json but can be used by Magento modules
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-63054: [Catalog] MSRP field is not displayed for bundle products with fixed price
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@okorshenko okorshenko

Labels
None yet
Projects
None yet
Milestone
June 2017
Development

Successfully merging this pull request may close these issues.
3 participants
@hostep @okorshenko @magento-team