Skip to content

Commit 25bc40e

Browse files
committed
MAGETWO-65458: Cart Rules are not excluding Bundle Products
1 parent 5565a24 commit 25bc40e

File tree

10 files changed

+313
-4
lines changed

10 files changed

+313
-4
lines changed

app/code/Magento/Bundle/composer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
},
2626
"suggest": {
2727
"magento/module-webapi": "*",
28-
"magento/module-bundle-sample-data": "*"
28+
"magento/module-bundle-sample-data": "*",
29+
"magento/module-sales-rule": "101.0.*"
2930
},
3031
"type": "magento2-module",
3132
"license": [

app/code/Magento/Bundle/etc/di.xml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,4 +207,11 @@
207207
</argument>
208208
</arguments>
209209
</type>
210+
<type name="Magento\SalesRule\Model\Quote\ChildrenValidationLocator">
211+
<arguments>
212+
<argument name="productTypeChildrenValidationMap" xsi:type="array">
213+
<item name="bundle" xsi:type="boolean">false</item>
214+
</argument>
215+
</arguments>
216+
</type>
210217
</config>
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\SalesRule\Model\Quote;
10+
11+
use Magento\Quote\Model\Quote\Item\AbstractItem as QuoteItem;
12+
13+
/**
14+
* Used to determine necessity to validate rule on item's children that may depends on product type.
15+
*/
16+
class ChildrenValidationLocator
17+
{
18+
/**
19+
* @var array
20+
*/
21+
private $productTypeChildrenValidationMap;
22+
23+
/**
24+
* @param array $productTypeChildrenValidationMap
25+
* <pre>
26+
* [
27+
* 'ProductType1' => true,
28+
* 'ProductType2' => false
29+
* ]
30+
* </pre>
31+
*/
32+
public function __construct(
33+
array $productTypeChildrenValidationMap = []
34+
) {
35+
$this->productTypeChildrenValidationMap = $productTypeChildrenValidationMap;
36+
}
37+
38+
/**
39+
* Checks necessity to validate rule on item's children.
40+
*
41+
* @param QuoteItem $item
42+
* @return bool
43+
*/
44+
public function isChildrenValidationRequired(QuoteItem $item): bool
45+
{
46+
$type = $item->getProduct()->getTypeId();
47+
if (isset($this->productTypeChildrenValidationMap[$type])) {
48+
return (bool)$this->productTypeChildrenValidationMap[$type];
49+
}
50+
51+
return true;
52+
}
53+
}

app/code/Magento/SalesRule/Model/RulesApplier.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
namespace Magento\SalesRule\Model;
77

88
use Magento\Quote\Model\Quote\Address;
9+
use Magento\SalesRule\Model\Quote\ChildrenValidationLocator;
10+
use Magento\Framework\App\ObjectManager;
11+
use Magento\SalesRule\Model\Rule\Action\Discount\CalculatorFactory;
912

1013
/**
1114
* Class RulesApplier
@@ -25,19 +28,33 @@ class RulesApplier
2528
*/
2629
protected $validatorUtility;
2730

31+
/**
32+
* @var ChildrenValidationLocator
33+
*/
34+
private $childrenValidationLocator;
35+
36+
/**
37+
* @var CalculatorFactory
38+
*/
39+
private $calculatorFactory;
40+
2841
/**
2942
* @param \Magento\SalesRule\Model\Rule\Action\Discount\CalculatorFactory $calculatorFactory
3043
* @param \Magento\Framework\Event\ManagerInterface $eventManager
3144
* @param \Magento\SalesRule\Model\Utility $utility
45+
* @param ChildrenValidationLocator $childrenValidationLocator
3246
*/
3347
public function __construct(
3448
\Magento\SalesRule\Model\Rule\Action\Discount\CalculatorFactory $calculatorFactory,
3549
\Magento\Framework\Event\ManagerInterface $eventManager,
36-
\Magento\SalesRule\Model\Utility $utility
50+
\Magento\SalesRule\Model\Utility $utility,
51+
ChildrenValidationLocator $childrenValidationLocator = null
3752
) {
3853
$this->calculatorFactory = $calculatorFactory;
3954
$this->validatorUtility = $utility;
4055
$this->_eventManager = $eventManager;
56+
$this->childrenValidationLocator = $childrenValidationLocator
57+
?: ObjectManager::getInstance()->get(ChildrenValidationLocator::class);
4158
}
4259

4360
/**
@@ -61,6 +78,9 @@ public function applyRules($item, $rules, $skipValidation, $couponCode)
6178
}
6279

6380
if (!$skipValidation && !$rule->getActions()->validate($item)) {
81+
if (!$this->childrenValidationLocator->isChildrenValidationRequired($item)) {
82+
continue;
83+
}
6484
$childItems = $item->getChildren();
6585
$isContinue = true;
6686
if (!empty($childItems)) {

app/code/Magento/SalesRule/Test/Unit/Model/RulesApplierTest.php

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
namespace Magento\SalesRule\Test\Unit\Model;
88

9+
/**
10+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
11+
*/
912
class RulesApplierTest extends \PHPUnit\Framework\TestCase
1013
{
1114
/**
@@ -28,6 +31,11 @@ class RulesApplierTest extends \PHPUnit\Framework\TestCase
2831
*/
2932
protected $validatorUtility;
3033

34+
/**
35+
* @var \Magento\SalesRule\Model\Quote\ChildrenValidationLocator|\PHPUnit_Framework_MockObject_MockObject
36+
*/
37+
protected $childrenValidationLocator;
38+
3139
protected function setUp()
3240
{
3341
$this->calculatorFactory = $this->createMock(
@@ -38,11 +46,15 @@ protected function setUp()
3846
\Magento\SalesRule\Model\Utility::class,
3947
['canProcessRule', 'minFix', 'deltaRoundingFix', 'getItemQty']
4048
);
41-
49+
$this->childrenValidationLocator = $this->createPartialMock(
50+
\Magento\SalesRule\Model\Quote\ChildrenValidationLocator::class,
51+
['isChildrenValidationRequired']
52+
);
4253
$this->rulesApplier = new \Magento\SalesRule\Model\RulesApplier(
4354
$this->calculatorFactory,
4455
$this->eventManager,
45-
$this->validatorUtility
56+
$this->validatorUtility,
57+
$this->childrenValidationLocator
4658
);
4759
}
4860

@@ -84,6 +96,10 @@ public function testApplyRulesWhenRuleWithStopRulesProcessingIsUsed($isChildren,
8496
$item->setDiscountCalculationPrice($positivePrice);
8597
$item->setData('calculation_price', $positivePrice);
8698

99+
$this->childrenValidationLocator->expects($this->any())
100+
->method('isChildrenValidationRequired')
101+
->willReturn(true);
102+
87103
$this->validatorUtility->expects($this->atLeastOnce())
88104
->method('canProcessRule')
89105
->will($this->returnValue(true));
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
use Magento\TestFramework\Helper\Bootstrap;
10+
11+
require __DIR__ . 'product_with_multiple_options.php';
12+
13+
$objectManager = Bootstrap::getObjectManager();
14+
15+
/** @var $product \Magento\Catalog\Model\Product */
16+
$product = $objectManager->create(\Magento\Catalog\Model\Product::class);
17+
$product->load(3);
18+
19+
/** @var $typeInstance \Magento\Bundle\Model\Product\Type */
20+
$typeInstance = $product->getTypeInstance();
21+
$typeInstance->setStoreFilter($product->getStoreId(), $product);
22+
$optionCollection = $typeInstance->getOptionsCollection($product);
23+
24+
$bundleOptions = [];
25+
$bundleOptionsQty = [];
26+
foreach ($optionCollection as $option) {
27+
/** @var $option \Magento\Bundle\Model\Option */
28+
$selectionsCollection = $typeInstance->getSelectionsCollection([$option->getId()], $product);
29+
if ($option->isMultiSelection()) {
30+
$bundleOptions[$option->getId()] = array_column($selectionsCollection->toArray(), 'selection_id');
31+
} else {
32+
$bundleOptions[$option->getId()] = $selectionsCollection->getFirstItem()->getSelectionId();
33+
}
34+
$bundleOptionsQty[$option->getId()] = 1;
35+
}
36+
37+
$requestInfo = new \Magento\Framework\DataObject(
38+
[
39+
'product' => $product->getId(),
40+
'bundle_option' => $bundleOptions,
41+
'bundle_option_qty' => $bundleOptionsQty,
42+
'qty' => 1,
43+
]
44+
);
45+
46+
/** @var $cart \Magento\Checkout\Model\Cart */
47+
$cart = Bootstrap::getObjectManager()->create(\Magento\Checkout\Model\Cart::class);
48+
$cart->addProduct($product, $requestInfo);
49+
$cart->getQuote()->setReservedOrderId('test_cart_with_bundle_and_options');
50+
$cart->save();
51+
52+
/** @var $objectManager \Magento\TestFramework\ObjectManager */
53+
$objectManager = Bootstrap::getObjectManager();
54+
$objectManager->removeSharedInstance(\Magento\Checkout\Model\Session::class);
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
/** @var \Magento\Framework\Registry $registry */
10+
$registry = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(\Magento\Framework\Registry::class);
11+
12+
$registry->unregister('isSecureArea');
13+
$registry->register('isSecureArea', true);
14+
15+
/** @var $objectManager \Magento\TestFramework\ObjectManager */
16+
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
17+
$quote = $objectManager->create(\Magento\Quote\Model\Quote::class);
18+
$quote->load('test_cart_with_bundle_and_options', 'reserved_order_id');
19+
$quote->delete();
20+
21+
/** @var \Magento\Quote\Model\QuoteIdMask $quoteIdMask */
22+
$quoteIdMask = $objectManager->create(\Magento\Quote\Model\QuoteIdMask::class);
23+
$quoteIdMask->delete($quote->getId());
24+
25+
require __DIR__ . 'product_with_multiple_options_rollback.php';
26+
27+
$registry->unregister('isSecureArea');
28+
$registry->register('isSecureArea', false);

dev/tests/integration/testsuite/Magento/SalesRule/Model/Rule/Condition/ProductTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,28 @@ public function testValidateCategorySalesRuleIncludesChildren($categoryId, $expe
5858
$this->assertEquals($expectedResult, $rule->validate($quote));
5959
}
6060

61+
/**
62+
* @magentoDbIsolation disabled
63+
* @magentoDataFixture Magento/Bundle/_files/order_item_with_bundle_and_options.php
64+
* @magentoDataFixture Magento/SalesRule/_files/rules_sku_exclude.php
65+
*
66+
* @return void
67+
*/
68+
public function testValidateSalesRuleExcludesBundleChildren(): void
69+
{
70+
// Load the quote that contains a child of a bundle product
71+
/** @var \Magento\Quote\Model\Quote $quote */
72+
$quote = $this->objectManager->create(\Magento\Quote\Model\Quote::class)
73+
->load('test_cart_with_bundle_and_options', 'reserved_order_id');
74+
75+
// Load the SalesRule looking for excluding products with selected sku
76+
/** @var $rule \Magento\SalesRule\Model\Rule */
77+
$rule = $this->objectManager->get(\Magento\Framework\Registry::class)
78+
->registry('_fixture/Magento_SalesRule_Sku_Exclude');
79+
80+
$this->assertEquals(false, $rule->validate($quote));
81+
}
82+
6183
/**
6284
* @return array
6385
*/
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
/** @var \Magento\Eav\Api\AttributeRepositoryInterface $repository */
10+
$repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()
11+
->create(\Magento\Eav\Api\AttributeRepositoryInterface::class);
12+
13+
/** @var \Magento\Eav\Api\Data\AttributeInterface $skuAttribute */
14+
$skuAttribute = $repository->get(
15+
'catalog_product',
16+
'sku'
17+
);
18+
$data = $skuAttribute->getData();
19+
$data['is_used_for_promo_rules'] = 1;
20+
$skuAttribute->setData($data);
21+
$skuAttribute->save();
22+
23+
/** @var \Magento\SalesRule\Model\Rule $rule */
24+
$salesRule = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(\Magento\SalesRule\Model\Rule::class);
25+
$salesRule->setData(
26+
[
27+
'name' => '20% Off',
28+
'is_active' => 1,
29+
'customer_group_ids' => [\Magento\Customer\Model\GroupManagement::NOT_LOGGED_IN_ID],
30+
'coupon_type' => \Magento\SalesRule\Model\Rule::COUPON_TYPE_NO_COUPON,
31+
'simple_action' => 'by_percent',
32+
'discount_amount' => 20,
33+
'discount_step' => 0,
34+
'stop_rules_processing' => 1,
35+
'website_ids' => [
36+
\Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
37+
\Magento\Store\Model\StoreManagerInterface::class
38+
)->getWebsite()->getId()
39+
],
40+
]
41+
);
42+
43+
$salesRule->getConditions()->loadArray([
44+
'type' => \Magento\SalesRule\Model\Rule\Condition\Combine::class,
45+
'attribute' => null,
46+
'operator' => null,
47+
'value' => '1',
48+
'is_value_processed' => null,
49+
'aggregator' => 'all',
50+
'conditions' =>
51+
[
52+
[
53+
'type' => \Magento\SalesRule\Model\Rule\Condition\Product\Found::class,
54+
'attribute' => null,
55+
'operator' => null,
56+
'value' => '1',
57+
'is_value_processed' => null,
58+
'aggregator' => 'all',
59+
'conditions' =>
60+
[
61+
[
62+
'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class,
63+
'attribute' => 'sku',
64+
'operator' => '!=',
65+
'value' => 'product-bundle',
66+
'is_value_processed' => false,
67+
],
68+
],
69+
],
70+
],
71+
]);
72+
73+
$salesRule->save();
74+
75+
/** @var Magento\Framework\Registry $registry */
76+
$registry = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(\Magento\Framework\Registry::class);
77+
78+
$registry->unregister('_fixture/Magento_SalesRule_Sku_Exclude');
79+
$registry->register('_fixture/Magento_SalesRule_Sku_Exclude', $salesRule);

0 commit comments

Comments
 (0)