Merge remote-tracking branch 'origin/MC-33313' into 2.4.1-develop-pr25

serhii-balko · serhii-balko · commit 15990d272b7d · 2020-05-18T09:58:04.000+03:00
diff --git a/app/code/Magento/Quote/Model/Quote/Address/Total/Subtotal.php b/app/code/Magento/Quote/Model/Quote/Address/Total/Subtotal.php
@@ -9,6 +9,9 @@
 use Magento\Quote\Model\Quote\Address\Item as AddressItem;
 use Magento\Quote\Model\Quote\Item;
 
+/**
+ * Address total collector model
+ */
 class Subtotal extends \Magento\Quote\Model\Quote\Address\Total\AbstractTotal
 {
     /**
@@ -89,40 +92,35 @@ protected function _initItem($address, $item)
         } else {
             $quoteItem = $item;
         }
-        $product = $quoteItem->getProduct();
-        $product->setCustomerGroupId($quoteItem->getQuote()->getCustomerGroupId());
-
-        /**
-         * Quote super mode flag mean what we work with quote without restriction
-         */
-        if ($item->getQuote()->getIsSuperMode()) {
-            if (!$product) {
-                return false;
-            }
-        } else {
-            if (!$product || !$product->isVisibleInCatalog()) {
-                return false;
+        $valid = false;
+        if ($quoteItem) {
+            $product = $quoteItem->getProduct();
+            /**
+             * Quote super mode flag mean what we work with quote without restriction
+             */
+            if ($product && ($item->getQuote()->getIsSuperMode() || $product->isVisibleInCatalog())) {
+                $product->setCustomerGroupId($quoteItem->getQuote()->getCustomerGroupId());
+                $quoteItem->setConvertedPrice(null);
+                $originalPrice = $product->getPrice();
+                if ($quoteItem->getParentItem() && $quoteItem->isChildrenCalculated()) {
+                    $finalPrice = $quoteItem->getParentItem()->getProduct()->getPriceModel()->getChildFinalPrice(
+                        $quoteItem->getParentItem()->getProduct(),
+                        $quoteItem->getParentItem()->getQty(),
+                        $product,
+                        $quoteItem->getQty()
+                    );
+                    $this->_calculateRowTotal($item, $finalPrice, $originalPrice);
+                } elseif (!$quoteItem->getParentItem()) {
+                    $finalPrice = $product->getFinalPrice($quoteItem->getQty());
+                    $this->_calculateRowTotal($item, $finalPrice, $originalPrice);
+                    $this->_addAmount($item->getRowTotal());
+                    $this->_addBaseAmount($item->getBaseRowTotal());
+                    $address->setTotalQty($address->getTotalQty() + $item->getQty());
+                }
+                $valid = true;
             }
         }
-
-        $quoteItem->setConvertedPrice(null);
-        $originalPrice = $product->getPrice();
-        if ($quoteItem->getParentItem() && $quoteItem->isChildrenCalculated()) {
-            $finalPrice = $quoteItem->getParentItem()->getProduct()->getPriceModel()->getChildFinalPrice(
-                $quoteItem->getParentItem()->getProduct(),
-                $quoteItem->getParentItem()->getQty(),
-                $product,
-                $quoteItem->getQty()
-            );
-            $this->_calculateRowTotal($item, $finalPrice, $originalPrice);
-        } elseif (!$quoteItem->getParentItem()) {
-            $finalPrice = $product->getFinalPrice($quoteItem->getQty());
-            $this->_calculateRowTotal($item, $finalPrice, $originalPrice);
-            $this->_addAmount($item->getRowTotal());
-            $this->_addBaseAmount($item->getBaseRowTotal());
-            $address->setTotalQty($address->getTotalQty() + $item->getQty());
-        }
-        return true;
+        return $valid;
     }
 
     /**
@@ -147,7 +145,7 @@ protected function _calculateRowTotal($item, $finalPrice, $originalPrice)
      * Remove item
      *
      * @param Address $address
-     * @param  AddressItem|Item $item
+     * @param AddressItem|Item $item
      * @return $this
      */
     protected function _removeItem($address, $item)
diff --git a/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php b/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php
@@ -1,5 +1,4 @@
 <?php
-
 /**
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
@@ -11,22 +10,23 @@
 use Magento\Catalog\Api\Data\ProductExtensionInterface;
 use Magento\Catalog\Model\Product;
 use Magento\Catalog\Model\Product\Type\Price;
-use Magento\CatalogInventory\Model\Stock\Item;
 use Magento\CatalogInventory\Model\StockRegistry;
 use Magento\Framework\Pricing\PriceCurrencyInterface;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use Magento\Quote\Api\Data\ShippingAssignmentInterface;
 use Magento\Quote\Api\Data\ShippingInterface;
 use Magento\Quote\Model\Quote;
 use Magento\Quote\Model\Quote\Address;
+use Magento\Quote\Model\Quote\Address\Item as AddressItem;
 use Magento\Quote\Model\Quote\Address\Total;
 use Magento\Quote\Model\Quote\Address\Total\Subtotal;
+use Magento\Quote\Model\Quote\Item;
 use Magento\Store\Model\Store;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
 /**
- * TODO refactor me
+ * Test address total collector model.
  *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
@@ -53,10 +53,18 @@ class SubtotalTest extends TestCase
     protected function setUp(): void
     {
         $this->objectManager = new ObjectManager($this);
-        $this->subtotalModel = $this->objectManager->getObject(Subtotal::class);
+        $this->subtotalModel = $this->objectManager->getObject(
+            Subtotal::class
+        );
 
-        $this->stockRegistry = $this->createPartialMock(StockRegistry::class, ['getStockItem']);
-        $this->stockItemMock = $this->createPartialMock(Item::class, ['getIsInStock']);
+        $this->stockRegistry = $this->createPartialMock(
+            StockRegistry::class,
+            ['getStockItem', '__wakeup']
+        );
+        $this->stockItemMock = $this->createPartialMock(
+            \Magento\CatalogInventory\Model\Stock\Item::class,
+            ['getIsInStock', '__wakeup']
+        );
     }
 
     /**
@@ -88,30 +96,28 @@ public function testCollect($price, $originalPrice, $itemHasParent, $expectedPri
     {
         $this->stockRegistry->expects($this->any())->method('getStockItem')->willReturn($this->stockItemMock);
 
-        $priceCurrency = $this->getMockBuilder(PriceCurrencyInterface::class)
-            ->getMock();
+        $priceCurrency = $this->getMockBuilder(PriceCurrencyInterface::class)->getMock();
         $convertedPrice = 1231313;
         // @TODO this is a wrong test and it does not check methods. Any digital value will be correct
         $priceCurrency->expects($this->any())->method('convert')->willReturn(1231313);
 
-        /** @var \Magento\Quote\Model\Quote\Item|MockObject $quoteItem */
+        /** @var Item|MockObject $quoteItem */
         $quoteItem = $this->objectManager->getObject(
-            \Magento\Quote\Model\Quote\Item::class,
+            Item::class,
             [
                 'stockRegistry' => $this->stockRegistry,
                 'priceCurrency' => $priceCurrency,
             ]
         );
         /** @var Address|MockObject $address */
-        $address = $this->getMockBuilder(Address::class)
-            ->addMethods(['setTotalQty', 'getTotalQty'])
-            ->onlyMethods(['removeItem', 'getQuote'])
-            ->disableOriginalConstructor()
-            ->getMock();
+        $address = $this->createPartialMock(
+            Address::class,
+            ['setTotalQty', 'getTotalQty', 'removeItem', 'getQuote']
+        );
 
         /** @var Product|MockObject $product */
         $product = $this->createMock(Product::class);
-        $product->expects($this->any())->method('getPrice')->willReturn($originalPrice);
+        $product->expects($this->any())->method('getPrice')->will($this->returnValue($originalPrice));
 
         /** @var Quote|MockObject $quote */
         $quote = $this->createMock(Quote::class);
@@ -121,22 +127,22 @@ public function testCollect($price, $originalPrice, $itemHasParent, $expectedPri
         $store = $this->createPartialMock(Store::class, ['getWebsiteId']);
         $store->expects($this->any())->method('getWebsiteId')->willReturn(10);
         $product->expects($this->any())->method('getStore')->willReturn($store);
-        $product->expects($this->any())->method('isVisibleInCatalog')->willReturn(true);
+        $product->expects($this->any())->method('isVisibleInCatalog')->will($this->returnValue(true));
         $extensionAttribute = $this->getMockBuilder(ProductExtensionInterface::class)
             ->setMethods(['getStockItem'])
             ->disableOriginalConstructor()
             ->getMockForAbstractClass();
         $extensionAttribute->expects($this->atLeastOnce())
             ->method('getStockItem')
-            ->willReturn($this->stockItemMock);
+            ->will($this->returnValue($this->stockItemMock));
         $product->expects($this->atLeastOnce())->method('getExtensionAttributes')->willReturn($extensionAttribute);
-        $quote->expects($this->any())->method('getStore')->willReturn($store);
+        $quote->expects($this->any())->method('getStore')->will($this->returnValue($store));
         $quoteItem->setProduct($product)->setQuote($quote);
 
         $parentQuoteItem = false;
         if ($itemHasParent) {
-            $parentQuoteItem = $this->createMock(\Magento\Quote\Model\Quote\Item::class);
-            $parentQuoteItem->expects($this->any())->method('getProduct')->willReturn($product);
+            $parentQuoteItem = $this->createMock(Item::class);
+            $parentQuoteItem->expects($this->any())->method('getProduct')->will($this->returnValue($product));
         }
         $quoteItem->setParentItem($parentQuoteItem);
         //This value will be overwritten
@@ -147,18 +153,18 @@ public function testCollect($price, $originalPrice, $itemHasParent, $expectedPri
         $product->expects($this->any())->method('getPriceModel')->willReturn($priceModel);
         $product->expects($this->any())->method('getFinalPrice')->willReturn($price);
 
-        $shipping = $this->getMockForAbstractClass(ShippingInterface::class);
+        $shipping = $this->createMock(ShippingInterface::class);
         $shipping->expects($this->exactly(2))->method('getAddress')->willReturn($address);
         $address->expects($this->at(0))->method('setTotalQty')->with(0);
         $address->expects($this->any())->method('getTotalQty')->willReturn(0);
-        $shippingAssignmentMock = $this->getMockForAbstractClass(ShippingAssignmentInterface::class);
+        $shippingAssignmentMock = $this->createMock(ShippingAssignmentInterface::class);
         $shippingAssignmentMock->expects($this->exactly(2))->method('getShipping')->willReturn($shipping);
         $shippingAssignmentMock->expects($this->once())->method('getItems')->willReturn([$quoteItem]);
 
-        $total = $this->getMockBuilder(Total::class)
-            ->addMethods(['setBaseVirtualAmount', 'setVirtualAmount'])
-            ->disableOriginalConstructor()
-            ->getMock();
+        $total = $this->createPartialMock(
+            Total::class,
+            ['setBaseVirtualAmount', 'setVirtualAmount']
+        );
         $total->expects($this->once())->method('setBaseVirtualAmount')->willReturnSelf();
         $total->expects($this->once())->method('setVirtualAmount')->willReturnSelf();
 
@@ -179,12 +185,74 @@ public function testFetch()
         ];
 
         $quoteMock = $this->createMock(Quote::class);
-        $totalMock = $this->getMockBuilder(Total::class)
-            ->addMethods(['getSubtotal'])
-            ->disableOriginalConstructor()
-            ->getMock();
+        $totalMock = $this->createPartialMock(Total::class, ['getSubtotal']);
         $totalMock->expects($this->once())->method('getSubtotal')->willReturn(100);
 
         $this->assertEquals($expectedResult, $this->subtotalModel->fetch($quoteMock, $totalMock));
     }
+
+    /**
+     * Test that invalid items are not collected
+     */
+    public function testCollectWithInvalidItems()
+    {
+        $addressItemId = 38203;
+        $addressQuoteItemId = 7643;
+        $storeId = 1;
+        $quote = $this->createPartialMock(
+            Quote::class,
+            [
+                'getItemsCollection',
+            ]
+        );
+        $quote->setData(
+            [
+                'store_id' => $storeId
+            ]
+        );
+        $quoteItem = $this->createPartialMock(
+            Item::class,
+            []
+        );
+        $quoteItem->setQuote($quote);
+        $quote->method('getItemsCollection')
+            ->willReturn([$quoteItem]);
+        $address = $this->createPartialMock(
+            Address::class,
+            [
+                'removeItem',
+                'getQuote'
+            ]
+        );
+        $address->method('getQuote')
+            ->willReturn($quote);
+        $address->expects($this->once())
+            ->method('removeItem')
+            ->with($addressItemId);
+        $addressItem = $this->createPartialMock(
+            AddressItem::class,
+            [
+                'getId',
+                'getQuoteItemId'
+            ]
+        );
+        $addressItem->setAddress($address);
+        $addressItem->method('getId')
+            ->willReturn($addressItemId);
+        $addressItem->method('getQuoteItemId')
+            ->willReturn($addressQuoteItemId);
+        $shipping = $this->createMock(ShippingInterface::class);
+        $shipping->method('getAddress')
+            ->willReturn($address);
+        $shippingAssignmentMock = $this->createMock(ShippingAssignmentInterface::class);
+        $shippingAssignmentMock->method('getShipping')
+            ->willReturn($shipping);
+        $shippingAssignmentMock->method('getItems')
+            ->willReturn([$addressItem]);
+        $total = $this->createPartialMock(
+            Total::class,
+            []
+        );
+        $this->subtotalModel->collect($quote, $shippingAssignmentMock, $total);
+    }
 }
