MAGETWO-93057: [2.3] salesShipmentRepositoryV1 throws error when adding tracking

serhii-balko · serhii-balko · commit 1ebe5f184724 · 2018-08-10T13:58:41.000+03:00
diff --git a/app/code/Magento/Sales/Model/Order/Shipment.php b/app/code/Magento/Sales/Model/Order/Shipment.php
@@ -9,6 +9,7 @@
 use Magento\Sales\Api\Data\ShipmentInterface;
 use Magento\Sales\Model\AbstractModel;
 use Magento\Sales\Model\EntityInterface;
+use Magento\Sales\Model\ResourceModel\Order\Shipment\Comment\Collection as CommentsCollection;
 
 /**
  * Sales order shipment model
@@ -94,9 +95,14 @@ class Shipment extends AbstractModel implements EntityInterface, ShipmentInterfa
     protected $orderRepository;
 
     /**
-     * @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Track\Collection|null
+     * @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Track\Collection
      */
-    private $tracksCollection = null;
+    private $tracksCollection;
+
+    /**
+     * @var CommentsCollection
+     */
+    private $commentsCollection;
 
     /**
      * @param \Magento\Framework\Model\Context $context
@@ -414,43 +420,45 @@ public function addTrack(\Magento\Sales\Model\Order\Shipment\Track $track)
     public function addComment($comment, $notify = false, $visibleOnFront = false)
     {
         if (!$comment instanceof \Magento\Sales\Model\Order\Shipment\Comment) {
-            $comment = $this->_commentFactory->create()->setComment(
-                $comment
-            )->setIsCustomerNotified(
-                $notify
-            )->setIsVisibleOnFront(
-                $visibleOnFront
-            );
+            $comment = $this->_commentFactory->create()
+                ->setComment($comment)
+                ->setIsCustomerNotified($notify)
+                ->setIsVisibleOnFront($visibleOnFront);
         }
-        $comment->setShipment($this)->setParentId($this->getId())->setStoreId($this->getStoreId());
+        $comment->setShipment($this)
+            ->setParentId($this->getId())
+            ->setStoreId($this->getStoreId());
         if (!$comment->getId()) {
             $this->getCommentsCollection()->addItem($comment);
         }
+        $comments = $this->getComments();
+        $comments[] = $comment;
+        $this->setComments($comments);
         $this->_hasDataChanges = true;
         return $this;
     }
 
     /**
-     * Retrieve comments collection.
+     * Retrieves comments collection.
      *
      * @param bool $reload
-     * @return \Magento\Sales\Model\ResourceModel\Order\Shipment\Comment\Collection
+     * @return CommentsCollection
      */
     public function getCommentsCollection($reload = false)
     {
-        if (!$this->hasData(ShipmentInterface::COMMENTS) || $reload) {
-            $comments = $this->_commentCollectionFactory->create()
-                ->setShipmentFilter($this->getId())
-                ->setCreatedAtOrder();
-            $this->setComments($comments);
-
+        if ($this->commentsCollection === null || $reload) {
+            $this->commentsCollection = $this->_commentCollectionFactory->create();
             if ($this->getId()) {
-                foreach ($this->getComments() as $comment) {
+                $this->commentsCollection->setShipmentFilter($this->getId())
+                    ->setCreatedAtOrder();
+
+                foreach ($this->commentsCollection as $comment) {
                     $comment->setShipment($this);
                 }
             }
         }
-        return $this->getComments();
+
+        return $this->commentsCollection;
     }
 
     /**
diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/ShipmentTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/ShipmentTest.php
@@ -25,7 +25,7 @@ class ShipmentTest extends \PHPUnit\Framework\TestCase
     private $commentCollection;
 
     /**
-     * @var \Magento\Sales\Model\Order\shipment
+     * @var Shipment
      */
     private $shipmentModel;
 
@@ -46,9 +46,6 @@ public function testGetIncrementId()
         $this->assertEquals('test_increment_id', $this->shipmentModel->getIncrementId());
     }
 
-    /**
-     * @covers \Magento\Sales\Model\Order\Shipment::getCommentsCollection
-     */
     public function testGetCommentsCollection()
     {
         $shipmentId = 1;
@@ -58,36 +55,29 @@ public function testGetCommentsCollection()
             ->disableOriginalConstructor()
             ->setMethods(['setShipment'])
             ->getMock();
-        $shipmentItem->expects(static::once())
-            ->method('setShipment')
+        $shipmentItem->method('setShipment')
             ->with($this->shipmentModel);
         $collection = [$shipmentItem];
 
-        $this->commentCollection->expects(static::once())
+        $this->commentCollection->expects(self::once())
             ->method('setShipmentFilter')
             ->with($shipmentId)
             ->willReturnSelf();
-        $this->commentCollection->expects(static::once())
+        $this->commentCollection->expects(self::once())
             ->method('setCreatedAtOrder')
             ->willReturnSelf();
 
-        $this->commentCollection->expects(static::once())
-            ->method('load')
-            ->willReturnSelf();
-
         $reflection = new \ReflectionClass(Collection::class);
         $reflectionProperty = $reflection->getProperty('_items');
         $reflectionProperty->setAccessible(true);
         $reflectionProperty->setValue($this->commentCollection, $collection);
 
-        $expected = $this->shipmentModel->getCommentsCollection();
+        $actual = $this->shipmentModel->getCommentsCollection();
 
-        static::assertEquals($expected, $this->commentCollection);
+        self::assertTrue(is_object($actual));
+        self::assertEquals($this->commentCollection, $actual);
     }
 
-    /**
-     * @covers \Magento\Sales\Model\Order\Shipment::getComments
-     */
     public function testGetComments()
     {
         $shipmentId = 1;
@@ -97,30 +87,27 @@ public function testGetComments()
             ->disableOriginalConstructor()
             ->setMethods(['setShipment'])
             ->getMock();
-        $shipmentItem->expects(static::once())
+        $shipmentItem->expects(self::once())
             ->method('setShipment')
             ->with($this->shipmentModel);
         $collection = [$shipmentItem];
 
-        $this->commentCollection->expects(static::once())
-            ->method('setShipmentFilter')
+        $this->commentCollection->method('setShipmentFilter')
             ->with($shipmentId)
             ->willReturnSelf();
 
-        $this->commentCollection->expects(static::once())
-            ->method('load')
-            ->willReturnSelf();
-
         $reflection = new \ReflectionClass(Collection::class);
         $reflectionProperty = $reflection->getProperty('_items');
         $reflectionProperty->setAccessible(true);
         $reflectionProperty->setValue($this->commentCollection, $collection);
 
-        $this->commentCollection->expects(static::once())
+        $this->commentCollection->expects(self::once())
             ->method('getItems')
             ->willReturn($collection);
 
-        static::assertEquals($this->shipmentModel->getComments(), $collection);
+        $actual = $this->shipmentModel->getComments();
+        self::assertTrue(is_array($actual));
+        self::assertEquals($collection, $actual);
     }
 
     /**
@@ -131,16 +118,15 @@ private function initCommentsCollectionFactoryMock()
     {
         $this->commentCollection = $this->getMockBuilder(Collection::class)
             ->disableOriginalConstructor()
-            ->setMethods(['setShipmentFilter', 'setCreatedAtOrder', 'getItems', 'load', '__wakeup'])
+            ->setMethods(['setShipmentFilter', 'setCreatedAtOrder', 'getItems', 'load'])
             ->getMock();
 
         $this->commentCollectionFactory = $this->getMockBuilder(CollectionFactory::class)
             ->disableOriginalConstructor()
             ->setMethods(['create'])
             ->getMock();
 
-        $this->commentCollectionFactory->expects(static::any())
-            ->method('create')
+        $this->commentCollectionFactory->method('create')
             ->willReturn($this->commentCollection);
     }
 }
diff --git a/dev/tests/integration/testsuite/Magento/Sales/Model/Order/ShipmentTest.php b/dev/tests/integration/testsuite/Magento/Sales/Model/Order/ShipmentTest.php
@@ -3,83 +3,147 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
 
 namespace Magento\Sales\Model\Order;
 
+use Magento\Framework\Api\SearchCriteriaBuilder;
+use Magento\Payment\Helper\Data;
+use Magento\Sales\Api\Data\CommentInterface;
+use Magento\Sales\Api\Data\OrderInterface;
+use Magento\Sales\Api\Data\ShipmentInterface;
+use Magento\Sales\Api\Data\ShipmentItemInterface;
+use Magento\Sales\Api\Data\ShipmentTrackInterface;
+use Magento\Sales\Api\OrderRepositoryInterface;
+use Magento\Sales\Api\ShipmentRepositoryInterface;
+use Magento\TestFramework\Helper\Bootstrap;
+use Magento\TestFramework\ObjectManager;
+
 /**
- * Class ShipmentTest
  * @magentoAppIsolation enabled
- * @package Magento\Sales\Model\Order
+ * @magentoDataFixture Magento/Sales/_files/order.php
  */
 class ShipmentTest extends \PHPUnit\Framework\TestCase
 {
+    /**
+     * @var ObjectManager
+     */
+    private $objectManager;
+
+    /**
+     * @var ShipmentRepositoryInterface
+     */
+    private $shipmentRepository;
+
+    /**
+     * @inheritdoc
+     */
+    protected function setUp()
+    {
+        $this->objectManager = Bootstrap::getObjectManager();
+        $this->shipmentRepository = $this->objectManager->get(ShipmentRepositoryInterface::class);
+    }
+
     /**
      * Check the correctness and stability of set/get packages of shipment
      *
-     * @magentoDataFixture Magento/Sales/_files/order.php
+     * @magentoAppArea frontend
      */
     public function testPackages()
     {
-        $objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
-        $objectManager->get(\Magento\Framework\App\State::class)->setAreaCode('frontend');
-        $order = $objectManager->create(\Magento\Sales\Model\Order::class);
-        $order->loadByIncrementId('100000001');
-        $order->setCustomerEmail('customer@example.com');
+        $order = $this->getOrder('100000001');
 
         $payment = $order->getPayment();
-        $paymentInfoBlock = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
-            \Magento\Payment\Helper\Data::class
-        )->getInfoBlock(
-            $payment
-        );
+        $paymentInfoBlock = $this->objectManager->get(Data::class)
+            ->getInfoBlock($payment);
         $payment->setBlockMock($paymentInfoBlock);
 
         $items = [];
         foreach ($order->getItems() as $item) {
             $items[$item->getId()] = $item->getQtyOrdered();
         }
         /** @var \Magento\Sales\Model\Order\Shipment $shipment */
-        $shipment = $objectManager->get(ShipmentFactory::class)->create($order, $items);
+        $shipment = $this->objectManager->get(ShipmentFactory::class)->create($order, $items);
 
         $packages = [['1'], ['2']];
 
         $shipment->setPackages($packages);
-        $this->assertEquals($packages, $shipment->getPackages());
-        $shipment->save();
-        $shipment->save();
-        $shipment->load($shipment->getId());
-        $this->assertEquals($packages, $shipment->getPackages());
+        $saved = $this->shipmentRepository->save($shipment);
+        self::assertEquals($packages, $saved->getPackages());
     }
 
     /**
      * Check that getTracksCollection() always return collection instance.
-     *
-     * @magentoDataFixture Magento/Sales/_files/order.php
      */
     public function testAddTrack()
     {
-        $objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
+        $order = $this->getOrder('100000001');
 
-        $order = $objectManager->create(\Magento\Sales\Model\Order::class);
-        $order->loadByIncrementId('100000001');
+        /** @var ShipmentInterface $shipment */
+        $shipment = $this->objectManager->create(ShipmentInterface::class);
+
+        /** @var ShipmentTrackInterface $track */
+        $track = $this->objectManager->create(ShipmentTrackInterface::class);
+        $track->setNumber('Test Number')
+            ->setTitle('Test Title')
+            ->setCarrierCode('Test CODE');
 
         $items = [];
         foreach ($order->getItems() as $item) {
             $items[$item->getId()] = $item->getQtyOrdered();
         }
         /** @var \Magento\Sales\Model\Order\Shipment $shipment */
-        $shipment = $objectManager->get(ShipmentFactory::class)->create($order, $items);
+        $shipment = $this->objectManager->get(ShipmentFactory::class)->create($order, $items);
+        $shipment->addTrack($track);
         $shipment->save();
+        $saved = $this->shipmentRepository->save($shipment);
+        self::assertNotEmpty($saved->getTracks());
+    }
+
+    /**
+     * Checks adding comment to the shipment entity.
+     */
+    public function testAddComment()
+    {
+        $message1 = 'Test Comment 1';
+        $message2 = 'Test Comment 2';
+        $order = $this->getOrder('100000001');
+
+        /** @var ShipmentInterface $shipment */
+        $shipment = $this->objectManager->create(ShipmentInterface::class);
+        $shipment->setOrder($order)
+            ->addItem($this->objectManager->create(ShipmentItemInterface::class))
+            ->addComment($message1)
+            ->addComment($message2);
 
-        /** @var $track \Magento\Sales\Model\Order\Shipment\Track */
-        $track = $objectManager->get(\Magento\Sales\Model\Order\Shipment\Track::class);
-        $track->setNumber('Test Number')->setTitle('Test Title')->setCarrierCode('Test CODE');
+        $saved = $this->shipmentRepository->save($shipment);
+
+        $comments = $saved->getComments();
+        $actual = array_map(function (CommentInterface $comment) {
+            return $comment->getComment();
+        }, $comments);
+        self::assertEquals(2, count($actual));
+        self::assertEquals([$message1, $message2], $actual);
+    }
+
+    /**
+     * Gets order entity by increment id.
+     *
+     * @param string $incrementId
+     * @return OrderInterface
+     */
+    private function getOrder(string $incrementId): OrderInterface
+    {
+        /** @var SearchCriteriaBuilder $searchCriteriaBuilder */
+        $searchCriteriaBuilder = $this->objectManager->get(SearchCriteriaBuilder::class);
+        $searchCriteria = $searchCriteriaBuilder->addFilter('increment_id', $incrementId)
+            ->create();
 
-        $this->assertEmpty($shipment->getTracks());
-        $shipment->addTrack($track)->save();
+        /** @var OrderRepositoryInterface $repository */
+        $repository = $this->objectManager->get(OrderRepositoryInterface::class);
+        $items = $repository->getList($searchCriteria)
+            ->getItems();
 
-        //to empty cache
-        $shipment->setTracks(null);
-        $this->assertNotEmpty($shipment->getTracks());
+        return array_pop($items);
     }
 }
