Skip to content

#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does… #16342

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
90b6803
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Jun 22, 2018
2717cb1
Removed empty line due to failing static test
phoenix128 Jun 25, 2018
d2a0de8
Removed dependency to configurable product
phoenix128 Jun 26, 2018
fae98c0
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Jul 4, 2018
9a35b45
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Jul 4, 2018
887ee4a
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Jul 7, 2018
0c9aa81
Revert "#14020-Cart-Sales-Rule-with-negated-condition-over-special-pr…
novikor Jul 22, 2018
4e68337
Merge branch '2.2-develop' into #14020-Cart-Sales-Rule-with-negated-c…
novikor Jul 22, 2018
5b95b22
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Jul 22, 2018
618f408
Moved adjustments to plugin.
novikor Jul 22, 2018
7c8482b
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Jul 23, 2018
e7130bf
Merge remote-tracking branch 'upstream/2.2-develop' into #14020-Cart-…
novikor Aug 9, 2018
5a7e78a
Merge remote-tracking branch 'origin/#14020-Cart-Sales-Rule-with-nega…
novikor Aug 9, 2018
8d417ac
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Aug 9, 2018
90ff989
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Aug 9, 2018
c52e0e8
Covered case when simple product is being validated
novikor Aug 9, 2018
5c3154b
#14020-Cart-Sales-Rule-with-negated-condition-over-special-price-does…
novikor Aug 9, 2018
51adb9d
Merge remote-tracking branch 'upstream/2.2-develop' into #14020-Cart-…
novikor Aug 12, 2018
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
@@ -0,0 +1,53 @@
<?php declare(strict_types=1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, cover new code at least by the unit test. It would be great if scenario will be covered with integration test

Copy link
Contributor Author

@novikor novikor Aug 10, 2018

Choose a reason for hiding this comment

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

Unit tests coverage have been implemented.
@sidolov, please check.


namespace Magento\ConfigurableProduct\Plugin\SalesRule\Model\Rule\Condition;

use Magento\ConfigurableProduct\Model\Product\Type\Configurable;

/**
* Class Product
*
* @package Magento\ConfigurableProduct\Plugin\SalesRule\Model\Rule\Condition
*/
class Product
{
/**
* @param \Magento\SalesRule\Model\Rule\Condition\Product $subject
* @param \Magento\Framework\Model\AbstractModel $model
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, remove extra spaces

*/
public function beforeValidate(
\Magento\SalesRule\Model\Rule\Condition\Product $subject,
\Magento\Framework\Model\AbstractModel $model
) {
$model->setProduct(
$this->getProductToValidate($subject, $model)
);
}

/**
* @param \Magento\SalesRule\Model\Rule\Condition\Product $subject
* @param \Magento\Framework\Model\AbstractModel $model
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, remove extra spaces

*
* @return \Magento\Catalog\Api\Data\ProductInterface|\Magento\Catalog\Model\Product
*/
private function getProductToValidate(
\Magento\SalesRule\Model\Rule\Condition\Product $subject,
\Magento\Framework\Model\AbstractModel $model
) {
/** @var \Magento\Catalog\Model\Product $product */
$product = $model->getProduct();

$attrCode = $subject->getAttribute();

/* Check for attributes which are not available for configurable products */
if ($product->getTypeId() == Configurable::TYPE_CODE && !$product->hasData($attrCode)) {
/** @var \Magento\Catalog\Model\AbstractModel $childProduct */
$childProduct = current($model->getChildren())->getProduct();
if ($childProduct->hasData($attrCode)) {
$product = $childProduct;
}
}

return $product;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
<?php declare(strict_types=1);
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\ConfigurableProduct\Test\Unit\Plugin\SalesRule\Model\Rule\Condition;

use Magento\Backend\Helper\Data;
use Magento\Catalog\Api\ProductRepositoryInterface;
use Magento\Catalog\Model\Product\Type;
use Magento\Catalog\Model\ProductFactory;
use Magento\Catalog\Model\ResourceModel\Product;
use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
use Magento\ConfigurableProduct\Plugin\SalesRule\Model\Rule\Condition\Product as ValidatorPlugin;
use Magento\Directory\Model\CurrencyFactory;
use Magento\Eav\Model\Config;
use Magento\Eav\Model\Entity\AbstractEntity;
use Magento\Eav\Model\ResourceModel\Entity\Attribute\Set\Collection;
use Magento\Framework\App\ScopeResolverInterface;
use Magento\Framework\Locale\Format;
use Magento\Framework\Locale\FormatInterface;
use Magento\Framework\Locale\ResolverInterface;
use Magento\Quote\Model\Quote\Item\AbstractItem;
use Magento\Rule\Model\Condition\Context;
use Magento\SalesRule\Model\Rule\Condition\Product as SalesRuleProduct;

/**
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
* @SuppressWarnings(PHPMD.LongVariable)
*/
class ProductTest extends \PHPUnit\Framework\TestCase
{
/**
* @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
*/
private $objectManager;

/**
* @var SalesRuleProduct
*/
private $validator;

/**
* @var \Magento\ConfigurableProduct\Plugin\SalesRule\Model\Rule\Condition\Product
*/
private $validatorPlugin;

public function setUp()
{
$this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
$this->validator = $this->createValidator();
$this->validatorPlugin = $this->objectManager->getObject(ValidatorPlugin::class);
}

/**
* @return \Magento\SalesRule\Model\Rule\Condition\Product
*/
private function createValidator(): SalesRuleProduct
{
/** @var Context|\PHPUnit_Framework_MockObject_MockObject $contextMock */
$contextMock = $this->getMockBuilder(Context::class)
->disableOriginalConstructor()
->getMock();
/** @var Data|\PHPUnit_Framework_MockObject_MockObject $backendHelperMock */
$backendHelperMock = $this->getMockBuilder(Data::class)
->disableOriginalConstructor()
->getMock();
/** @var Config|\PHPUnit_Framework_MockObject_MockObject $configMock */
$configMock = $this->getMockBuilder(Config::class)
->disableOriginalConstructor()
->getMock();
/** @var ProductFactory|\PHPUnit_Framework_MockObject_MockObject $productFactoryMock */
$productFactoryMock = $this->getMockBuilder(ProductFactory::class)
->disableOriginalConstructor()
->getMock();
/** @var ProductRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject $productRepositoryMock */
$productRepositoryMock = $this->getMockBuilder(ProductRepositoryInterface::class)
->getMockForAbstractClass();
$attributeLoaderInterfaceMock = $this->getMockBuilder(AbstractEntity::class)
->disableOriginalConstructor()
->setMethods(['getAttributesByCode'])
->getMock();
$attributeLoaderInterfaceMock
->expects($this->any())
->method('getAttributesByCode')
->willReturn([]);
/** @var Product|\PHPUnit_Framework_MockObject_MockObject $productMock */
$productMock = $this->getMockBuilder(Product::class)
->disableOriginalConstructor()
->setMethods(['loadAllAttributes', 'getConnection', 'getTable'])
->getMock();
$productMock->expects($this->any())
->method('loadAllAttributes')
->willReturn($attributeLoaderInterfaceMock);
/** @var Collection|\PHPUnit_Framework_MockObject_MockObject $collectionMock */
$collectionMock = $this->getMockBuilder(Collection::class)
->disableOriginalConstructor()
->getMock();
/** @var FormatInterface|\PHPUnit_Framework_MockObject_MockObject $formatMock */
$formatMock = new Format(
$this->getMockBuilder(ScopeResolverInterface::class)->disableOriginalConstructor()->getMock(),
$this->getMockBuilder(ResolverInterface::class)->disableOriginalConstructor()->getMock(),
$this->getMockBuilder(CurrencyFactory::class)->disableOriginalConstructor()->getMock()
);

return new SalesRuleProduct(
$contextMock,
$backendHelperMock,
$configMock,
$productFactoryMock,
$productRepositoryMock,
$productMock,
$collectionMock,
$formatMock
);
}

public function testChildIsUsedForValidation()
{
$configurableProductMock = $this->createProductMock();
$configurableProductMock
->expects($this->any())
->method('getTypeId')
->willReturn(Configurable::TYPE_CODE);
$configurableProductMock
->expects($this->any())
->method('hasData')
->with($this->equalTo('special_price'))
->willReturn(false);

/* @var AbstractItem|\PHPUnit_Framework_MockObject_MockObject $item */
$item = $this->getMockBuilder(AbstractItem::class)
->disableOriginalConstructor()
->setMethods(['setProduct', 'getProduct', 'getChildren'])
->getMockForAbstractClass();
$item->expects($this->any())
->method('getProduct')
->willReturn($configurableProductMock);

$simpleProductMock = $this->createProductMock();
$simpleProductMock
->expects($this->any())
->method('getTypeId')
->willReturn(Type::TYPE_SIMPLE);
$simpleProductMock
->expects($this->any())
->method('hasData')
->with($this->equalTo('special_price'))
->willReturn(true);

$childItem = $this->getMockBuilder(AbstractItem::class)
->disableOriginalConstructor()
->setMethods(['getProduct'])
->getMockForAbstractClass();
$childItem->expects($this->any())
->method('getProduct')
->willReturn($simpleProductMock);

$item->expects($this->any())
->method('getChildren')
->willReturn([$childItem]);
$item->expects($this->once())
->method('setProduct')
->with($this->identicalTo($simpleProductMock));

$this->validator->setAttribute('special_price');

$this->validatorPlugin->beforeValidate($this->validator, $item);
}

/**
* @return Product|\PHPUnit_Framework_MockObject_MockObject
*/
private function createProductMock(): \PHPUnit_Framework_MockObject_MockObject
{
$productMock = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
->disableOriginalConstructor()
->setMethods([
'getAttribute',
'getId',
'setQuoteItemQty',
'setQuoteItemPrice',
'getTypeId',
'hasData',
])
->getMock();
$productMock
->expects($this->any())
->method('setQuoteItemQty')
->willReturnSelf();
$productMock
->expects($this->any())
->method('setQuoteItemPrice')
->willReturnSelf();

return $productMock;
}

public function testChildIsNotUsedForValidation()
{
$simpleProductMock = $this->createProductMock();
$simpleProductMock
->expects($this->any())
->method('getTypeId')
->willReturn(Type::TYPE_SIMPLE);
$simpleProductMock
->expects($this->any())
->method('hasData')
->with($this->equalTo('special_price'))
->willReturn(true);

/* @var AbstractItem|\PHPUnit_Framework_MockObject_MockObject $item */
$item = $this->getMockBuilder(AbstractItem::class)
->disableOriginalConstructor()
->setMethods(['setProduct', 'getProduct'])
->getMockForAbstractClass();
$item->expects($this->any())
->method('getProduct')
->willReturn($simpleProductMock);

$item->expects($this->once())
->method('setProduct')
->with($this->identicalTo($simpleProductMock));

$this->validator->setAttribute('special_price');

$this->validatorPlugin->beforeValidate($this->validator, $item);
}
}
1 change: 1 addition & 0 deletions app/code/Magento/ConfigurableProduct/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"suggest": {
"magento/module-webapi": "100.2.*",
"magento/module-sales": "101.0.*",
"magento/module-sales-rule": "101.0.*",
"magento/module-product-video": "100.2.*",
"magento/module-configurable-sample-data": "Sample Data version:100.2.*",
"magento/module-product-links-sample-data": "Sample Data version:100.2.*"
Expand Down
3 changes: 3 additions & 0 deletions app/code/Magento/ConfigurableProduct/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@
<type name="Magento\Catalog\Model\Product">
<plugin name="product_identities_extender" type="Magento\ConfigurableProduct\Model\Plugin\ProductIdentitiesExtender" />
</type>
<type name="Magento\SalesRule\Model\Rule\Condition\Product">
<plugin name="apply_rule_on_configurable_children" type="Magento\ConfigurableProduct\Plugin\SalesRule\Model\Rule\Condition\Product" />
</type>
<type name="Magento\Catalog\Model\Product\Configuration\Item\ItemResolverComposite">
<arguments>
<argument name="itemResolvers" xsi:type="array">
Expand Down
4 changes: 4 additions & 0 deletions app/code/Magento/SalesRule/Model/Rule/Condition/Product.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
* Product rule condition data model
*
* @author Magento Core Team <[email protected]>
*
* @method string getAttribute()
*/
class Product extends \Magento\Rule\Model\Condition\Product\AbstractProduct
{
Expand All @@ -31,7 +33,9 @@ protected function _addSpecialAttributes(array &$attributes)
* Validate Product Rule Condition
*
* @param \Magento\Framework\Model\AbstractModel $model
*
* @return bool
* @throws \Magento\Framework\Exception\NoSuchEntityException
*/
public function validate(\Magento\Framework\Model\AbstractModel $model)
{
Expand Down