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

Elasticsearch price fieldname is incorrect during indexing when storeId and websiteId do not match #21216

Merged
merged 2 commits into from
Apr 5, 2019

Conversation

alexander-aleman
Copy link
Contributor

Description (*)

The method getProductPriceData expects $websiteId as a parameter, but $storeId was given. It should be $websiteId since prices are website specific. Also see \Magento\Elasticsearch\Elasticsearch5\Model\Adapter\DataMapper\ProductDataMapper where a similar method exists and $websiteId is used. In cases where $websiteId and $storeId are different this causes the price filter to not work because ElasticSearch contains a field $fieldName = 'price_' . $customerGroupId . '' . $storeId, but during filtering $fieldName = 'price' . $customerGroupId . '_' . $websiteId is requested.

Fixed Issues (if relevant)

Manual testing scenarios (*)

  1. Create a website with 2 stores (i.e. there is a store with storeId which differs from websiteId)
  2. Enable elasticsearch as search backend
  3. Enable index on schedule
  4. run bin/magento indexer:reindex
  5. Make sure the price attribute is enabled as a filter
  6. The price filter should now work for both stores after this change

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)

The method getProductPriceData expects $websiteId as a parameter, but $storeId was given. It should be $websiteId since prices are website specific. Also see \Magento\Elasticsearch\Elasticsearch5\Model\Adapter\DataMapper\ProductDataMapper where a similar method exists and $websiteId is used.
@magento-engcom-team
Copy link
Contributor

Hi @alexander-aleman. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Feb 14, 2019

CLA assistant check
All committers have signed the CLA.

@aleron75 aleron75 self-assigned this Feb 14, 2019
@magento-engcom-team
Copy link
Contributor

Hi @aleron75, thank you for the review.
ENGCOM-4268 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@alexander-aleman thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

merge from magento/magento2
@m2-assistant
Copy link

m2-assistant bot commented Apr 5, 2019

Hi @alexander-aleman, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento-engcom-team pushed a commit that referenced this pull request Apr 5, 2019
@magento-engcom-team magento-engcom-team added this to the Release: 2.3.2 milestone Apr 5, 2019
unicoder88 added a commit to ConvertGroupsAS/magento2-patches that referenced this pull request Sep 22, 2019
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.

6 participants