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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Magento\Catalog\Model\ResourceModel\Product\LinkedProductSelectBuilderInterface;
use Magento\Framework\App\ResourceConnection;
use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory;
use Magento\Store\Model\StoreManagerInterface;

/**
* Retrieve list of products where each product contains lower price than others at least for one possible price type
Expand All @@ -31,7 +32,12 @@ class LowestPriceOptionsProvider implements LowestPriceOptionsProviderInterface
private $collectionFactory;

/**
* Key is product id. Value is array of prepared linked products
* @var StoreManagerInterface
*/
private $storeManager;

/**
* Key is product id and store id. Value is array of prepared linked products
*
* @var array
*/
Expand All @@ -41,34 +47,38 @@ class LowestPriceOptionsProvider implements LowestPriceOptionsProviderInterface
* @param ResourceConnection $resourceConnection
* @param LinkedProductSelectBuilderInterface $linkedProductSelectBuilder
* @param CollectionFactory $collectionFactory
* @param StoreManagerInterface $storeManager
*/
public function __construct(
ResourceConnection $resourceConnection,
LinkedProductSelectBuilderInterface $linkedProductSelectBuilder,
CollectionFactory $collectionFactory
CollectionFactory $collectionFactory,
StoreManagerInterface $storeManager
) {
$this->resource = $resourceConnection;
$this->linkedProductSelectBuilder = $linkedProductSelectBuilder;
$this->collectionFactory = $collectionFactory;
$this->storeManager = $storeManager;
}

/**
* {@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

if (!isset($this->linkedProductMap[$key])) {
$productIds = $this->resource->getConnection()->fetchCol(
'(' . implode(') UNION (', $this->linkedProductSelectBuilder->build($product->getId())) . ')'
);

$this->linkedProductMap[$product->getId()] = $this->collectionFactory->create()
$this->linkedProductMap[$key] = $this->collectionFactory->create()
->addAttributeToSelect(
['price', 'special_price', 'special_from_date', 'special_to_date', 'tax_class_id']
)
->addIdFilter($productIds)
->getItems();
}
return $this->linkedProductMap[$product->getId()];
return $this->linkedProductMap[$key];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
use Magento\Catalog\Api\Data\ProductInterface;
use Magento\Catalog\Model\ResourceModel\Product\LinkedProductSelectBuilderInterface;
use Magento\Store\Model\StoreManagerInterface;
use Magento\Store\Api\Data\StoreInterface;
use Magento\Store\Model\Store;

class LowestPriceOptionsProviderTest extends \PHPUnit\Framework\TestCase
{
Expand Down Expand Up @@ -42,6 +45,16 @@ class LowestPriceOptionsProviderTest extends \PHPUnit\Framework\TestCase
*/
private $productCollection;

/**
* @var StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $storeManagerMock;

/**
* @var StoreInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $storeMock;

protected function setUp()
{
$this->connection = $this
Expand All @@ -68,6 +81,11 @@ protected function setUp()
->setMethods(['create'])
->getMock();
$this->collectionFactory->expects($this->once())->method('create')->willReturn($this->productCollection);
$this->storeManagerMock = $this->getMockBuilder(StoreManagerInterface::class)
->getMockForAbstractClass();
$this->storeMock = $this->getMockBuilder(StoreInterface::class)
->setMethods(['getId'])
->getMockForAbstractClass();

$objectManager = new ObjectManager($this);
$this->model = $objectManager->getObject(
Expand All @@ -76,6 +94,7 @@ protected function setUp()
'resourceConnection' => $this->resourceConnection,
'linkedProductSelectBuilder' => $this->linkedProductSelectBuilder,
'collectionFactory' => $this->collectionFactory,
'storeManager' => $this->storeManagerMock,
]
);
}
Expand All @@ -94,6 +113,13 @@ public function testGetProducts()
->willReturnSelf();
$this->productCollection->expects($this->once())->method('addIdFilter')->willReturnSelf();
$this->productCollection->expects($this->once())->method('getItems')->willReturn($linkedProducts);
$this->storeManagerMock->expects($this->any())
->method('getStore')
->with(Store::DEFAULT_STORE_ID)
->willReturn($this->storeMock);
$this->storeMock->expects($this->any())
->method('getId')
->willReturn(Store::DEFAULT_STORE_ID);

$this->assertEquals($linkedProducts, $this->model->getProducts($product));
}
Expand Down