diff --git a/app/code/Magento/SalesRule/Model/Validator.php b/app/code/Magento/SalesRule/Model/Validator.php index dad35165051ce..cdaac97fe6fb5 100644 --- a/app/code/Magento/SalesRule/Model/Validator.php +++ b/app/code/Magento/SalesRule/Model/Validator.php @@ -383,13 +383,7 @@ public function initTotals($items, Address $address) foreach ($items as $item) { //Skipping child items to avoid double calculations - if ($item->getParentItemId()) { - continue; - } - if (!$rule->getActions()->validate($item)) { - continue; - } - if (!$this->canApplyDiscount($item)) { + if (!$this->isValidItemForRule($item, $rule)) { continue; } $qty = $this->validatorUtility->getItemQty($item, $rule); @@ -409,6 +403,30 @@ public function initTotals($items, Address $address) return $this; } + /** + * Determine if quote item is valid for a given sales rule + * + * @param AbstractItem $item + * @param Rule $rule + * @return bool + */ + private function isValidItemForRule(AbstractItem $item, Rule $rule) + { + if ($item->getParentItemId()) { + return false; + } + if ($item->getParentItem()) { + return false; + } + if (!$rule->getActions()->validate($item)) { + return false; + } + if (!$this->canApplyDiscount($item)) { + return false; + } + return true; + } + /** * Return item price * diff --git a/app/code/Magento/SalesRule/Test/Unit/Model/ValidatorTest.php b/app/code/Magento/SalesRule/Test/Unit/Model/ValidatorTest.php index e86068946ca78..946963dd8d234 100644 --- a/app/code/Magento/SalesRule/Test/Unit/Model/ValidatorTest.php +++ b/app/code/Magento/SalesRule/Test/Unit/Model/ValidatorTest.php @@ -13,7 +13,7 @@ use PHPUnit\Framework\MockObject\MockObject; /** - * Class ValidatorTest + * Tests for Magento\SalesRule\Model\Validator * @@SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class ValidatorTest extends \PHPUnit\Framework\TestCase @@ -346,11 +346,14 @@ public function testInitTotalsCanApplyDiscount() 'getDiscountCalculationPrice', 'getBaseDiscountCalculationPrice', 'getCalculationPrice', - 'getParentItemId' + 'getParentItemId', + 'getParentItem' ] ); $item2 = clone $item1; - $items = [$item1, $item2]; + $item3 = clone $item1; + $item4 = clone $item1; + $items = [$item1, $item2, $item3, $item4]; $rule->expects($this->any()) ->method('getSimpleAction') @@ -367,12 +370,22 @@ public function testInitTotalsCanApplyDiscount() $validator->expects($this->at(0))->method('isValid')->with($item1)->willReturn(false); $validator->expects($this->at(1))->method('isValid')->with($item2)->willReturn(true); - $item1->expects($this->any())->method('getParentItemId')->willReturn(false); + $item1->expects($this->any())->method('getParentItemId')->willReturn(null); + $item1->expects($this->any())->method('getParentItem')->willReturn(null); $item1->expects($this->never())->method('getDiscountCalculationPrice'); $item1->expects($this->never())->method('getBaseDiscountCalculationPrice'); - $item2->expects($this->any())->method('getParentItemId')->willReturn(false); + $item2->expects($this->any())->method('getParentItemId')->willReturn(null); + $item2->expects($this->any())->method('getParentItem')->willReturn(null); $item2->expects($this->any())->method('getDiscountCalculationPrice')->willReturn(50); $item2->expects($this->once())->method('getBaseDiscountCalculationPrice')->willReturn(50); + $item3->expects($this->any())->method('getParentItemId')->willReturn(null); + $item3->expects($this->any())->method('getParentItem')->willReturn($item1); + $item3->expects($this->never())->method('getDiscountCalculationPrice'); + $item3->expects($this->never())->method('getBaseDiscountCalculationPrice'); + $item4->expects($this->any())->method('getParentItemId')->willReturn(12345); + $item4->expects($this->any())->method('getParentItem')->willReturn(null); + $item4->expects($this->never())->method('getDiscountCalculationPrice'); + $item4->expects($this->never())->method('getBaseDiscountCalculationPrice'); $this->utility->expects($this->once())->method('getItemQty')->willReturn(1); $this->utility->expects($this->any())->method('canProcessRule')->willReturn(true);