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

Configurable product price options by store #13933

Conversation

simpleadm
Copy link
Contributor

@simpleadm simpleadm commented Mar 3, 2018

Added compatability of LowestPriceOptionsProvider class and environmen emulation fucntionality.
Updated linkedProductMap property in lowest price options provider to cache options by store and product id.

Description

Some modules (e.g. Abandoned Cart, Algolia Search etc.) use store emulation functionality (\Magento\Store\Model\App\Emulation::startEnvironmentEmulation) to get product info (including configurable product price) for each store. Unfortunately LowestPriceOptionsProvider class save linked products collection at the $linkedProductMap property based on requested product id only. That is why configurable product price will be the same for other stores after first emulation.

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)

Forwardport for: #13808

}

/**
* {@inheritdoc}
*/
public function getProducts(ProductInterface $product)
{
if (!isset($this->linkedProductMap[$product->getId()])) {
$key = $this->storeManager->getStore()->getId() . '-' . $product->getId();
Copy link
Contributor

Choose a reason for hiding this comment

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

Price of product in Magento can be modified only on website level, so better use website id here

Copy link
Contributor Author

@simpleadm simpleadm Mar 5, 2018

Choose a reason for hiding this comment

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

You are right, prices and tax class id can be modified only on website level, but scope for 'Special Price From Date' and 'Special Price To Date' can be changed from admin panel http://prntscr.com/in1pba . So that attribute can affect results. That's why i decided to use store id.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that scope for 'Special Price From Date' is wrong. All price related option should be a website or global level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are totally right, but we have what we have and that works fine. If we will change for website level - in environmen emulation we will get different results than on real website. That why i propose to keep it without changes.

I tried to create 3 products: 2 simple, 1 configurable and set 'Special Price From Date' for store view level.
Results:
Default store view: http://prntscr.com/ipevz0
Second store view: http://prntscr.com/ipewma

Product settings:
http://prntscr.com/ipex7e (Second store view)
http://prntscr.com/ipexld (all stores and default store view is without overrides)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On my opinion, store view level caching shouldn't affect performance.

If i remember correctly 'Special Price From Date' attribute doesn't exist in price index, that why it could be in store view level and works correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The last word is yours ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

In any case, your solution fixes the problem and improves the system, and my comment is just a suggestion for improvement, you can ignore it as a minor

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.

4 participants