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

Fix typo in SQL join when joining custom option prices for price indexer #19546

Conversation

udovicic
Copy link
Member

@udovicic udovicic commented Dec 4, 2018

Description (*)

In case where product has price variation for custom option per website, only default value (store_id = 0) is taken into an account due to typo in SQL statement.

Fixed Issues (if relevant)

  1. N/A

Manual testing scenarios (*)

This one is pretty obvious from diff, but...

  1. Create scenario where simple product price is regulated per website and have at least two websites
  2. Set default price to amount X
  3. Set price of custom option (on default view) to amount to Y
  4. Switch scope to another website and set price of custom option to be 0
  5. Execute price indexer
  6. Difference between min and max price should be visible only on one website, however all websites will have this difference

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)

@magento-engcom-team
Copy link
Contributor

Hi @udovicic. 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

@udovicic udovicic changed the title Fix typo in SQL join Fix typo in SQL join when joining custom option prices for price indexer Dec 4, 2018
@orlangur orlangur self-assigned this Dec 4, 2018
@magento-engcom-team magento-engcom-team added this to the Release: 2.3.1 milestone Dec 4, 2018
@magento-engcom-team
Copy link
Contributor

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

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.

@udovicic, QA report follows:

Not clear what issue should be caused by typo in DefaultPrice.php on 2.3-develop.
Manual testing scenario:

Create two websites: Main Website and Second
Set Catalog Price Scope=Website
Create simple product on both websites.
Add custom option:
Option Title=Option1
Option Type=Drop-Down
Title=Value
Price=10
Price Type=Fixed
Click Edit product
Change Store View to Second website store view
Set price of custom option to be 0
Execute price indexer: php bin/magento indexer:reindex catalog_product_price
Go to Product Page
 Product and custom option prices are correct displayed

Go to Product Page on another webiste
 Product and custom option prices are correct displayed

Please provide more details so that issue can be reproduced.

@udovicic
Copy link
Member Author

Hi @orlangur ,

You are correct, I have tested on wrong version and therefore patched the wrong file. Apologize for that. Nevertheless, issue persist as the bug was just migrated to new file (not sure what the old one is for, but I left change there as well).

Hover, steps to reproduce are not entirely correct. When I said "min and max price should be visible only on one website", I didn't mean on frontend but in the database in the table catalog_product_index_price actual min_price and max_price fields. Should have been more clear on that.

If you check the setup of that product you will see that only thing that influences price of product is a custom option. Therefore, for website where custom option has a price >0, max_price value in the database table catalog_product_index_price should be larger than final_price. In contrast to that, on website where custom option price is set to 0, there should be no difference between those two. Without the modifications in this PR, that is not the case but is expected result.

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.

@udovicic thanks, let's recheck.

@magento-engcom-team
Copy link
Contributor

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

@okorshenko okorshenko removed this from the Release: 2.3.1 milestone Jan 28, 2019
@magento-engcom-team magento-engcom-team merged commit c1be7d8 into magento:2.3-develop Feb 13, 2019
@ghost
Copy link

ghost commented Feb 13, 2019

Hi @udovicic, 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.

@sidolov sidolov added this to the Release: 2.3.2 milestone Feb 13, 2019
@udovicic udovicic deleted the bugfix/price-indexer-custom-options branch February 18, 2019 07:11
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