MAGETWO-87291: #7760: M2.1.2 : Shipment Tracking REST API should thro…

…w an error if order doesn't exist. #1162

app/code/Magento/Sales/Model/Order/Shipment/TrackRepository.php

@@ -14,6 +14,7 @@
 use Magento\Sales\Api\Data\ShipmentTrackSearchResultInterfaceFactory;
 use Magento\Sales\Api\ShipmentTrackRepositoryInterface;
 use Magento\Sales\Model\Spi\ShipmentTrackResourceInterface;
+use Psr\Log\LoggerInterface;
 
 class TrackRepository implements ShipmentTrackRepositoryInterface
 {
@@ -37,23 +38,30 @@ class TrackRepository implements ShipmentTrackRepositoryInterface
      */
     private $collectionProcessor;
 
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
     /**
      * @param ShipmentTrackResourceInterface $trackResource
      * @param ShipmentTrackInterfaceFactory $trackFactory
      * @param ShipmentTrackSearchResultInterfaceFactory $searchResultFactory
      * @param CollectionProcessorInterface $collectionProcessor
+     * @param LoggerInterface|null $logger
      */
     public function __construct(
         ShipmentTrackResourceInterface $trackResource,
         ShipmentTrackInterfaceFactory $trackFactory,
         ShipmentTrackSearchResultInterfaceFactory $searchResultFactory,
-        CollectionProcessorInterface $collectionProcessor
+        CollectionProcessorInterface $collectionProcessor,
+        LoggerInterface $logger = null
     ) {
-
         $this->trackResource = $trackResource;
         $this->trackFactory = $trackFactory;
         $this->searchResultFactory = $searchResultFactory;
         $this->collectionProcessor = $collectionProcessor;
+        $this->logger = $logger ?: \Magento\Framework\App\ObjectManager::getInstance()->get(LoggerInterface::class);
     }
 
     /**
@@ -64,6 +72,7 @@ public function getList(SearchCriteriaInterface $searchCriteria)
         $searchResult = $this->searchResultFactory->create();
         $this->collectionProcessor->process($searchCriteria, $searchResult);
         $searchResult->setSearchCriteria($searchCriteria);
+
         return $searchResult;
     }
 
@@ -74,6 +83,7 @@ public function get($id)
     {
         $entity = $this->trackFactory->create();
         $this->trackResource->load($entity, $id);
+
         return $entity;
     }
 
@@ -85,8 +95,10 @@ public function delete(ShipmentTrackInterface $entity)
         try {
             $this->trackResource->delete($entity);
         } catch (\Exception $e) {
+            $this->logger->error($e->getMessage());
             throw new CouldNotDeleteException(__('Could not delete the shipment tracking.'), $e);
         }
+
         return true;
     }
 
@@ -98,8 +110,10 @@ public function save(ShipmentTrackInterface $entity)
         try {
             $this->trackResource->save($entity);
         } catch (\Exception $e) {
+            $this->logger->error($e->getMessage());
             throw new CouldNotSaveException(__('Could not save the shipment tracking.'), $e);
         }
+
         return $entity;
     }
 
@@ -109,6 +123,7 @@ public function save(ShipmentTrackInterface $entity)
     public function deleteById($id)
     {
         $entity = $this->get($id);
+
         return $this->delete($entity);
     }
 }

app/code/Magento/Sales/Model/ResourceModel/Order/Shipment/Track.php

@@ -5,6 +5,7 @@
  */
 namespace Magento\Sales\Model\ResourceModel\Order\Shipment;
 
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Sales\Model\ResourceModel\EntityAbstract as SalesResource;
 use Magento\Framework\Model\ResourceModel\Db\VersionControl\Snapshot;
 use Magento\Sales\Model\Spi\ShipmentTrackResourceInterface;
@@ -74,7 +75,7 @@ protected function _construct()
      *
      * @param \Magento\Framework\Model\AbstractModel $object
      * @return $this
-     * @throws \Magento\Framework\Exception\LocalizedException
+     * @throws LocalizedException
      */
     protected function _beforeSave(\Magento\Framework\Model\AbstractModel $object)
     {
@@ -86,11 +87,16 @@ protected function _beforeSave(\Magento\Framework\Model\AbstractModel $object)
         parent::_beforeSave($object);
         $errors = $this->validator->validate($object);
         if (!empty($errors)) {
-            throw new \Magento\Framework\Exception\LocalizedException(
+            throw new LocalizedException(
                 __("Cannot save track:\n%1", implode("\n", $errors))
             );
         }
 
+        if ($object->getShipment()->getOrder()->getId() != $object->getOrderId()) {
+            $errorMessage = 'Shipment with requested ID %1 doesn\'t correspond with Order with requested ID %2.';
+            throw new LocalizedException(__($errorMessage, $object->getParentId(), $object->getOrderId()));
+        }
+
         return $this;
     }
 }

app/code/Magento/Sales/Test/Unit/Model/ResourceModel/Order/Shipment/TrackTest.php

@@ -89,6 +89,8 @@ protected function setUp()
      */
     public function testSave()
     {
+        $shipmentMock = $this->createMock(\Magento\Sales\Model\Order\Shipment::class);
+        $orderMock = $this->createMock(\Magento\Sales\Model\Order::class);
         $this->entitySnapshotMock->expects($this->once())
             ->method('isModified')
             ->with($this->trackModelMock)
@@ -98,6 +100,8 @@ public function testSave()
             ->with($this->equalTo($this->trackModelMock))
             ->will($this->returnValue([]));
         $this->trackModelMock->expects($this->any())->method('getData')->willReturn([]);
+        $this->trackModelMock->expects($this->atLeastOnce())->method('getShipment')->willReturn($shipmentMock);
+        $shipmentMock->expects($this->atLeastOnce())->method('getOrder')->willReturn($orderMock);
         $this->trackResource->save($this->trackModelMock);
         $this->assertTrue(true);
     }

app/code/Magento/Sales/i18n/en_US.csv

@@ -803,3 +803,4 @@ Created,Created
 "PDF Shipments","PDF Shipments"
 "PDF Creditmemos","PDF Creditmemos"
 Refunds,Refunds
+"Shipment with requested ID %1 doesn't correspond with Order with requested ID %2.","Shipment with requested ID %1 doesn't correspond with Order with requested ID %2."

dev/tests/api-functional/testsuite/Magento/Sales/Service/V1/ShipmentAddTrackTest.php

@@ -74,6 +74,36 @@ public function testShipmentAddTrack()
         self::assertNotEmpty($result[ShipmentTrackInterface::ENTITY_ID]);
         self::assertEquals($shipment->getId(), $result[ShipmentTrackInterface::PARENT_ID]);
     }
+
+    /**
+     * Try to create track with wrong order ID.
+     *
+     * @magentoApiDataFixture Magento/Sales/_files/shipment.php
+     *
+     * @expectedException \Exception
+     * @expectedExceptionMessage Could not save the shipment tracking.
+     */
+    public function testShipmentAddTrackThrowsError()
+    {
+        $shipmentCollection = $this->objectManager->get(Collection::class);
+        /** @var \Magento\Sales\Model\Order\Shipment $shipment */
+        $shipment = $shipmentCollection->getFirstItem();
+
+        $trackData = [
+            ShipmentTrackInterface::ENTITY_ID => null,
+            ShipmentTrackInterface::ORDER_ID => $shipment->getOrderId() + 1,
+            ShipmentTrackInterface::PARENT_ID => $shipment->getId(),
+            ShipmentTrackInterface::WEIGHT => 20,
+            ShipmentTrackInterface::QTY => 5,
+            ShipmentTrackInterface::TRACK_NUMBER => 2,
+            ShipmentTrackInterface::DESCRIPTION => 'Shipment description',
+            ShipmentTrackInterface::TITLE => 'Shipment title',
+            ShipmentTrackInterface::CARRIER_CODE => Track::CUSTOM_CARRIER_CODE,
+        ];
+
+        $this->_webApiCall($this->getServiceInfo(), ['entity' => $trackData]);
+    }
+
     /**
      * Returns details about API endpoints and services.
      *