MAGETWO-96218: [2.3] Track not saved during shipment creation through…

… API - Refactored `getTracks` method - Fixed relation processor - Updated unit and integration tests
magento · Nov 9, 2018 · 2307e16 · 2307e16
1 parent 6481cec
commit 2307e16
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 102 deletions.
diff --git a/app/code/Magento/Sales/Model/Order/Shipment.php b/app/code/Magento/Sales/Model/Order/Shipment.php
@@ -354,7 +354,15 @@ public function addItem(\Magento\Sales\Model\Order\Shipment\Item $item)
     public function getTracksCollection()
     {
         if ($this->tracksCollection === null) {
-            $this->tracksCollection = $this->_trackCollectionFactory->create()->setShipmentFilter($this->getId());
+            $this->tracksCollection = $this->_trackCollectionFactory->create();
+
+            if ($this->getId()) {
+                $this->tracksCollection->setShipmentFilter($this->getId());
+
+                foreach ($this->tracksCollection as $item) {
+                    $item->setShipment($this);
+                }
+            }
         }
 
         return $this->tracksCollection;
@@ -400,19 +408,20 @@ public function getTrackById($trackId)
      */
     public function addTrack(\Magento\Sales\Model\Order\Shipment\Track $track)
     {
-        $track->setShipment(
-            $this
-        )->setParentId(
-            $this->getId()
-        )->setOrderId(
-            $this->getOrderId()
-        )->setStoreId(
-            $this->getStoreId()
-        );
+        $track->setShipment($this)
+            ->setParentId($this->getId())
+            ->setOrderId($this->getOrderId())
+            ->setStoreId($this->getStoreId());
+
         if (!$track->getId()) {
             $this->getTracksCollection()->addItem($track);
         }
 
+        $tracks = $this->getTracks();
+        // as it's a new track entity, the collection doesn't contain it
+        $tracks[] = $track;
+        $this->setTracks($tracks);
+
         /**
          * Track saving is implemented in _afterSave()
          * This enforces \Magento\Framework\Model\AbstractModel::save() not to skip _afterSave()
@@ -582,14 +591,15 @@ public function setItems($items)
     /**
      * Returns tracks
      *
-     * @return \Magento\Sales\Api\Data\ShipmentTrackInterface[]
+     * @return \Magento\Sales\Api\Data\ShipmentTrackInterface[]|null
      */
     public function getTracks()
     {
+        if (!$this->getId()) {
+            return $this->getData(ShipmentInterface::TRACKS);
+        }
+
         if ($this->getData(ShipmentInterface::TRACKS) === null) {
-            foreach ($this->getTracksCollection() as $item) {
-                $item->setShipment($this);
-            }
             $this->setData(ShipmentInterface::TRACKS, $this->getTracksCollection()->getItems());
         }
         return $this->getData(ShipmentInterface::TRACKS);

diff --git a/app/code/Magento/Sales/Model/ResourceModel/Order/Shipment/Relation.php b/app/code/Magento/Sales/Model/ResourceModel/Order/Shipment/Relation.php
@@ -62,8 +62,8 @@ public function processRelation(\Magento\Framework\Model\AbstractModel $object)
                 $this->shipmentItemResource->save($item);
             }
         }
-        if (null !== $object->getTracksCollection()) {
-            foreach ($object->getTracksCollection() as $track) {
+        if (null !== $object->getTracks()) {
+            foreach ($object->getTracks() as $track) {
                 $track->setParentId($object->getId());
                 $this->shipmentTrackResource->save($track);
             }

diff --git a/app/code/Magento/Sales/Test/Unit/Model/ResourceModel/Order/Shipment/RelationTest.php b/app/code/Magento/Sales/Test/Unit/Model/ResourceModel/Order/Shipment/RelationTest.php
@@ -3,145 +3,125 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
 
 namespace Magento\Sales\Test\Unit\Model\ResourceModel\Order\Shipment;
 
+use Magento\Sales\Model\Order\Shipment;
+use Magento\Sales\Model\Order\Shipment\Comment as CommentEntity;
+use Magento\Sales\Model\Order\Shipment\Item as ItemEntity;
+use Magento\Sales\Model\Order\Shipment\Track as TrackEntity;
+use Magento\Sales\Model\ResourceModel\Order\Shipment\Comment;
+use Magento\Sales\Model\ResourceModel\Order\Shipment\Item;
+use Magento\Sales\Model\ResourceModel\Order\Shipment\Relation;
+use Magento\Sales\Model\ResourceModel\Order\Shipment\Track;
+use PHPUnit\Framework\MockObject\MockObject;
+
 /**
  * Class RelationTest
  */
 class RelationTest extends \PHPUnit\Framework\TestCase
 {
     /**
-     * @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Relation
+     * @var Relation
      */
-    protected $relationProcessor;
+    private $relationProcessor;
 
     /**
-     * @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Item|\PHPUnit_Framework_MockObject_MockObject
+     * @var Item|MockObject
      */
-    protected $itemResourceMock;
+    private $itemResource;
 
     /**
-     * @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Track|\PHPUnit_Framework_MockObject_MockObject
+     * @var Track|MockObject
      */
-    protected $trackResourceMock;
+    private $trackResource;
 
     /**
-     * @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Comment|\PHPUnit_Framework_MockObject_MockObject
+     * @var Comment|MockObject
      */
-    protected $commentResourceMock;
+    private $commentResource;
 
     /**
-     * @var \Magento\Sales\Model\Order\Shipment\Comment|\PHPUnit_Framework_MockObject_MockObject
+     * @var CommentEntity|MockObject
      */
-    protected $commentMock;
+    private $comment;
 
     /**
-     * @var \Magento\Sales\Model\Order\Shipment\Track|\PHPUnit_Framework_MockObject_MockObject
+     * @var TrackEntity|MockObject
      */
-    protected $trackMock;
+    private $track;
 
     /**
-     * @var \Magento\Sales\Model\Order\Shipment|\PHPUnit_Framework_MockObject_MockObject
+     * @var Shipment|MockObject
      */
-    protected $shipmentMock;
+    private $shipment;
 
     /**
-     * @var \Magento\Sales\Model\Order\Shipment\Item|\PHPUnit_Framework_MockObject_MockObject
+     * @var ItemEntity|MockObject
      */
-    protected $itemMock;
+    private $item;
 
-    protected function setUp()
+    /**
+     * @inheritdoc
+     */
+    protected function setUp(): void
     {
-        $this->itemResourceMock = $this->getMockBuilder(\Magento\Sales\Model\ResourceModel\Order\Shipment\Item::class)
+        $this->itemResource = $this->getMockBuilder(Item::class)
             ->disableOriginalConstructor()
-            ->setMethods(
-                [
-                    'save'
-                ]
-            )
             ->getMock();
-        $this->commentResourceMock = $this->getMockBuilder(
-            \Magento\Sales\Model\ResourceModel\Order\Shipment\Comment::class
-        )
+        $this->commentResource = $this->getMockBuilder(Comment::class)
             ->disableOriginalConstructor()
-            ->setMethods(
-                [
-                    'save'
-                ]
-            )
             ->getMock();
-        $this->trackResourceMock = $this->getMockBuilder(\Magento\Sales\Model\ResourceModel\Order\Shipment\Track::class)
+        $this->trackResource = $this->getMockBuilder(Track::class)
             ->disableOriginalConstructor()
-            ->setMethods(
-                [
-                    'save'
-                ]
-            )
             ->getMock();
-        $this->shipmentMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment::class)
+        $this->shipment = $this->getMockBuilder(Shipment::class)
             ->disableOriginalConstructor()
-            ->setMethods(
-                [
-                    'getId',
-                    'getItems',
-                    'getTracks',
-                    'getComments',
-                    'getTracksCollection',
-                ]
-            )
             ->getMock();
-        $this->itemMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Item::class)
+        $this->item = $this->getMockBuilder(ItemEntity::class)
             ->disableOriginalConstructor()
-            ->setMethods(
-                [
-                    'setParentId'
-                ]
-            )
             ->getMock();
-        $this->trackMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment\Track::class)
+        $this->track = $this->getMockBuilder(TrackEntity::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->commentMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment::class)
+        $this->comment = $this->getMockBuilder(Shipment::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->relationProcessor = new \Magento\Sales\Model\ResourceModel\Order\Shipment\Relation(
-            $this->itemResourceMock,
-            $this->trackResourceMock,
-            $this->commentResourceMock
+        $this->relationProcessor = new Relation(
+            $this->itemResource,
+            $this->trackResource,
+            $this->commentResource
         );
     }
 
-    public function testProcessRelations()
+    /**
+     * Checks saving shipment relations.
+     *
+     * @throws \Exception
+     */
+    public function testProcessRelations(): void
     {
-        $this->shipmentMock->expects($this->exactly(3))
-            ->method('getId')
+        $this->shipment->method('getId')
             ->willReturn('shipment-id-value');
-        $this->shipmentMock->expects($this->exactly(2))
-            ->method('getItems')
-            ->willReturn([$this->itemMock]);
-        $this->shipmentMock->expects($this->exactly(2))
-            ->method('getComments')
-            ->willReturn([$this->commentMock]);
-        $this->shipmentMock->expects($this->exactly(2))
-            ->method('getTracksCollection')
-            ->willReturn([$this->trackMock]);
-        $this->itemMock->expects($this->once())
-            ->method('setParentId')
+        $this->shipment->method('getItems')
+            ->willReturn([$this->item]);
+        $this->shipment->method('getComments')
+            ->willReturn([$this->comment]);
+        $this->shipment->method('getTracks')
+            ->willReturn([$this->track]);
+        $this->item->method('setParentId')
             ->with('shipment-id-value')
             ->willReturnSelf();
-        $this->itemResourceMock->expects($this->once())
-            ->method('save')
-            ->with($this->itemMock)
+        $this->itemResource->method('save')
+            ->with($this->item)
             ->willReturnSelf();
-        $this->commentResourceMock->expects($this->once())
-            ->method('save')
-            ->with($this->commentMock)
+        $this->commentResource->method('save')
+            ->with($this->comment)
             ->willReturnSelf();
-        $this->trackResourceMock->expects($this->once())
-            ->method('save')
-            ->with($this->trackMock)
+        $this->trackResource->method('save')
+            ->with($this->track)
             ->willReturnSelf();
-        $this->relationProcessor->processRelation($this->shipmentMock);
+        $this->relationProcessor->processRelation($this->shipment);
     }
 }
diff --git a/dev/tests/integration/testsuite/Magento/Sales/Model/Order/ShipmentTest.php b/dev/tests/integration/testsuite/Magento/Sales/Model/Order/ShipmentTest.php
@@ -90,10 +90,11 @@ public function testAddTrack()
             $items[$item->getId()] = $item->getQtyOrdered();
         }
         /** @var \Magento\Sales\Model\Order\Shipment $shipment */
-        $shipment = $this->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);
+        $this->shipmentRepository->save($shipment);
+        $saved = $this->shipmentRepository->get((int)$shipment->getEntityId());
         self::assertNotEmpty($saved->getTracks());
     }