Skip to content

Commit 4b3b669

Browse files
MAGETWO-96156: Track not saved during shipment creation through API
1 parent 1dcba87 commit 4b3b669

File tree

4 files changed

+98
-109
lines changed

4 files changed

+98
-109
lines changed

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

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,15 @@ public function addItem(\Magento\Sales\Model\Order\Shipment\Item $item)
343343
public function getTracksCollection()
344344
{
345345
if ($this->tracksCollection === null) {
346-
$this->tracksCollection = $this->_trackCollectionFactory->create()->setShipmentFilter($this->getId());
346+
$this->tracksCollection = $this->_trackCollectionFactory->create();
347+
348+
if ($this->getId()) {
349+
$this->tracksCollection->setShipmentFilter($this->getId());
350+
351+
foreach ($this->tracksCollection as $item) {
352+
$item->setShipment($this);
353+
}
354+
}
347355
}
348356

349357
return $this->tracksCollection;
@@ -383,19 +391,20 @@ public function getTrackById($trackId)
383391
*/
384392
public function addTrack(\Magento\Sales\Model\Order\Shipment\Track $track)
385393
{
386-
$track->setShipment(
387-
$this
388-
)->setParentId(
389-
$this->getId()
390-
)->setOrderId(
391-
$this->getOrderId()
392-
)->setStoreId(
393-
$this->getStoreId()
394-
);
394+
$track->setShipment($this)
395+
->setParentId($this->getId())
396+
->setOrderId($this->getOrderId())
397+
->setStoreId($this->getStoreId());
398+
395399
if (!$track->getId()) {
396400
$this->getTracksCollection()->addItem($track);
397401
}
398402

403+
$tracks = $this->getTracks();
404+
// as it new track entity, collection doesn't contain it
405+
$tracks[] = $track;
406+
$this->setTracks($tracks);
407+
399408
/**
400409
* Track saving is implemented in _afterSave()
401410
* This enforces \Magento\Framework\Model\AbstractModel::save() not to skip _afterSave()
@@ -562,18 +571,16 @@ public function setItems($items)
562571
/**
563572
* Returns tracks
564573
*
565-
* @return \Magento\Sales\Api\Data\ShipmentTrackInterface[]
574+
* @return \Magento\Sales\Api\Data\ShipmentTrackInterface[]|null
566575
*/
567576
public function getTracks()
568577
{
578+
if (!$this->getId()) {
579+
return $this->getData(ShipmentInterface::TRACKS);
580+
}
581+
569582
if ($this->getData(ShipmentInterface::TRACKS) === null) {
570-
$collection = $this->_trackCollectionFactory->create()->setShipmentFilter($this->getId());
571-
if ($this->getId()) {
572-
foreach ($collection as $item) {
573-
$item->setShipment($this);
574-
}
575-
$this->setData(ShipmentInterface::TRACKS, $collection->getItems());
576-
}
583+
$this->setData(ShipmentInterface::TRACKS, $this->getTracksCollection()->getItems());
577584
}
578585
return $this->getData(ShipmentInterface::TRACKS);
579586
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ public function processRelation(\Magento\Framework\Model\AbstractModel $object)
6262
$this->shipmentItemResource->save($item);
6363
}
6464
}
65-
if (null !== $object->getTracksCollection()) {
66-
foreach ($object->getTracksCollection() as $track) {
65+
if (null !== $object->getTracks()) {
66+
foreach ($object->getTracks() as $track) {
6767
$track->setParentId($object->getId());
6868
$this->shipmentTrackResource->save($track);
6969
}

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

Lines changed: 61 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -3,145 +3,125 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
67

78
namespace Magento\Sales\Test\Unit\Model\ResourceModel\Order\Shipment;
89

10+
use Magento\Sales\Model\Order\Shipment;
11+
use Magento\Sales\Model\Order\Shipment\Comment as CommentEntity;
12+
use Magento\Sales\Model\Order\Shipment\Item as ItemEntity;
13+
use Magento\Sales\Model\Order\Shipment\Track as TrackEntity;
14+
use Magento\Sales\Model\ResourceModel\Order\Shipment\Comment;
15+
use Magento\Sales\Model\ResourceModel\Order\Shipment\Item;
16+
use Magento\Sales\Model\ResourceModel\Order\Shipment\Relation;
17+
use Magento\Sales\Model\ResourceModel\Order\Shipment\Track;
18+
use PHPUnit\Framework\MockObject\MockObject;
19+
920
/**
1021
* Class RelationTest
1122
*/
1223
class RelationTest extends \PHPUnit\Framework\TestCase
1324
{
1425
/**
15-
* @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Relation
26+
* @var Relation
1627
*/
17-
protected $relationProcessor;
28+
private $relationProcessor;
1829

1930
/**
20-
* @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Item|\PHPUnit_Framework_MockObject_MockObject
31+
* @var Item|MockObject
2132
*/
22-
protected $itemResourceMock;
33+
private $itemResource;
2334

2435
/**
25-
* @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Track|\PHPUnit_Framework_MockObject_MockObject
36+
* @var Track|MockObject
2637
*/
27-
protected $trackResourceMock;
38+
private $trackResource;
2839

2940
/**
30-
* @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Comment|\PHPUnit_Framework_MockObject_MockObject
41+
* @var Comment|MockObject
3142
*/
32-
protected $commentResourceMock;
43+
private $commentResource;
3344

3445
/**
35-
* @var \Magento\Sales\Model\Order\Shipment\Comment|\PHPUnit_Framework_MockObject_MockObject
46+
* @var CommentEntity|MockObject
3647
*/
37-
protected $commentMock;
48+
private $comment;
3849

3950
/**
40-
* @var \Magento\Sales\Model\Order\Shipment\Track|\PHPUnit_Framework_MockObject_MockObject
51+
* @var TrackEntity|MockObject
4152
*/
42-
protected $trackMock;
53+
private $track;
4354

4455
/**
45-
* @var \Magento\Sales\Model\Order\Shipment|\PHPUnit_Framework_MockObject_MockObject
56+
* @var Shipment|MockObject
4657
*/
47-
protected $shipmentMock;
58+
private $shipment;
4859

4960
/**
50-
* @var \Magento\Sales\Model\Order\Shipment\Item|\PHPUnit_Framework_MockObject_MockObject
61+
* @var ItemEntity|MockObject
5162
*/
52-
protected $itemMock;
63+
private $item;
5364

65+
/**
66+
* @inheritdoc
67+
*/
5468
protected function setUp()
5569
{
56-
$this->itemResourceMock = $this->getMockBuilder(\Magento\Sales\Model\ResourceModel\Order\Shipment\Item::class)
70+
$this->itemResource = $this->getMockBuilder(Item::class)
5771
->disableOriginalConstructor()
58-
->setMethods(
59-
[
60-
'save'
61-
]
62-
)
6372
->getMock();
64-
$this->commentResourceMock = $this->getMockBuilder(
65-
\Magento\Sales\Model\ResourceModel\Order\Shipment\Comment::class
66-
)
73+
$this->commentResource = $this->getMockBuilder(Comment::class)
6774
->disableOriginalConstructor()
68-
->setMethods(
69-
[
70-
'save'
71-
]
72-
)
7375
->getMock();
74-
$this->trackResourceMock = $this->getMockBuilder(\Magento\Sales\Model\ResourceModel\Order\Shipment\Track::class)
76+
$this->trackResource = $this->getMockBuilder(Track::class)
7577
->disableOriginalConstructor()
76-
->setMethods(
77-
[
78-
'save'
79-
]
80-
)
8178
->getMock();
82-
$this->shipmentMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment::class)
79+
$this->shipment = $this->getMockBuilder(Shipment::class)
8380
->disableOriginalConstructor()
84-
->setMethods(
85-
[
86-
'getId',
87-
'getItems',
88-
'getTracks',
89-
'getComments',
90-
'getTracksCollection',
91-
]
92-
)
9381
->getMock();
94-
$this->itemMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Item::class)
82+
$this->item = $this->getMockBuilder(ItemEntity::class)
9583
->disableOriginalConstructor()
96-
->setMethods(
97-
[
98-
'setParentId'
99-
]
100-
)
10184
->getMock();
102-
$this->trackMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment\Track::class)
85+
$this->track = $this->getMockBuilder(TrackEntity::class)
10386
->disableOriginalConstructor()
10487
->getMock();
105-
$this->commentMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment::class)
88+
$this->comment = $this->getMockBuilder(Shipment::class)
10689
->disableOriginalConstructor()
10790
->getMock();
108-
$this->relationProcessor = new \Magento\Sales\Model\ResourceModel\Order\Shipment\Relation(
109-
$this->itemResourceMock,
110-
$this->trackResourceMock,
111-
$this->commentResourceMock
91+
$this->relationProcessor = new Relation(
92+
$this->itemResource,
93+
$this->trackResource,
94+
$this->commentResource
11295
);
11396
}
11497

98+
/**
99+
* Checks saving shipment relations.
100+
*
101+
* @throws \Exception
102+
*/
115103
public function testProcessRelations()
116104
{
117-
$this->shipmentMock->expects($this->exactly(3))
118-
->method('getId')
105+
$this->shipment->method('getId')
119106
->willReturn('shipment-id-value');
120-
$this->shipmentMock->expects($this->exactly(2))
121-
->method('getItems')
122-
->willReturn([$this->itemMock]);
123-
$this->shipmentMock->expects($this->exactly(2))
124-
->method('getComments')
125-
->willReturn([$this->commentMock]);
126-
$this->shipmentMock->expects($this->exactly(2))
127-
->method('getTracksCollection')
128-
->willReturn([$this->trackMock]);
129-
$this->itemMock->expects($this->once())
130-
->method('setParentId')
107+
$this->shipment->method('getItems')
108+
->willReturn([$this->item]);
109+
$this->shipment->method('getComments')
110+
->willReturn([$this->comment]);
111+
$this->shipment->method('getTracks')
112+
->willReturn([$this->track]);
113+
$this->item->method('setParentId')
131114
->with('shipment-id-value')
132115
->willReturnSelf();
133-
$this->itemResourceMock->expects($this->once())
134-
->method('save')
135-
->with($this->itemMock)
116+
$this->itemResource->method('save')
117+
->with($this->item)
136118
->willReturnSelf();
137-
$this->commentResourceMock->expects($this->once())
138-
->method('save')
139-
->with($this->commentMock)
119+
$this->commentResource->method('save')
120+
->with($this->comment)
140121
->willReturnSelf();
141-
$this->trackResourceMock->expects($this->once())
142-
->method('save')
143-
->with($this->trackMock)
122+
$this->trackResource->method('save')
123+
->with($this->track)
144124
->willReturnSelf();
145-
$this->relationProcessor->processRelation($this->shipmentMock);
125+
$this->relationProcessor->processRelation($this->shipment);
146126
}
147127
}

dev/tests/integration/testsuite/Magento/Sales/Model/Order/ShipmentTest.php

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,20 +77,22 @@ public function testAddTrack()
7777
{
7878
$order = $this->getOrder('100000001');
7979

80-
/** @var ShipmentInterface $shipment */
81-
$shipment = $this->objectManager->create(ShipmentInterface::class);
82-
8380
/** @var ShipmentTrackInterface $track */
8481
$track = $this->objectManager->create(ShipmentTrackInterface::class);
8582
$track->setNumber('Test Number')
8683
->setTitle('Test Title')
8784
->setCarrierCode('Test CODE');
8885

89-
$shipment->setOrder($order)
90-
->addItem($this->objectManager->create(ShipmentItemInterface::class))
91-
->addTrack($track);
92-
93-
$saved = $this->shipmentRepository->save($shipment);
86+
$items = [];
87+
foreach ($order->getItems() as $item) {
88+
$items[$item->getId()] = $item->getQtyOrdered();
89+
}
90+
/** @var \Magento\Sales\Model\Order\Shipment $shipment */
91+
$shipment = $this->objectManager->get(ShipmentFactory::class)
92+
->create($order, $items);
93+
$shipment->addTrack($track);
94+
$this->shipmentRepository->save($shipment);
95+
$saved = $this->shipmentRepository->get((int)$shipment->getEntityId());
9496
self::assertNotEmpty($saved->getTracks());
9597
}
9698

0 commit comments

Comments
 (0)