ENGCOM-6824: #26622 - Check quote item for parentItem instead of pare…

…ntItemId #26623 - Merge Pull Request #26623 from aligent-lturner/magento2-1:fix/GH26622_fixed_amount_discount - Merged commits: 1. e943bda 2. 199afbf 3. 90c3540 4. 5f4cc5f 5. 0e9c4ef 6. 2c961af 7. 75b03e0 8. a037edf
magento · Feb 7, 2020 · 6d0081d · 6d0081d
2 parents 847f941 + a037edf
commit 6d0081d
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 12 deletions.
diff --git 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
@@ -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);