Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 57 additions & 3 deletions src/App/Order/Model/PdkPhysicalProperties.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
use MyParcelNL\Pdk\Types\Service\TriStateService;

/**
* @property null|int $height
* @property null|int $length
* @property null|int $width
* @property null|int $height In centimeters, as entered by the merchant. Null when not filled in.
* @property null|int $length In centimeters, as entered by the merchant. Null when not filled in.
* @property null|int $width In centimeters, as entered by the merchant. Null when not filled in.
* @property int $initialWeight
* @property int $manualWeight
* @property int $totalWeight
Expand Down Expand Up @@ -55,6 +55,9 @@ class PdkPhysicalProperties extends Model
public function toStorableArray(): array
{
return Utils::filterNull([
'height' => $this->height,
'length' => $this->length,
'width' => $this->width,
'manualWeight' => TriStateService::INHERIT === $this->manualWeight ? null : $this->manualWeight,
]);
}
Expand All @@ -70,4 +73,55 @@ protected function getTotalWeightAttribute(): int

return $triStateService->resolve($this->manualWeight, $this->initialWeight);
}

/**
* @param mixed $value
*
* @return self
* @noinspection PhpUnused
*/
protected function setHeightAttribute($value): self
{
return $this->setDimension('height', $value);
}

/**
* @param mixed $value
*
* @return self
* @noinspection PhpUnused
*/
protected function setLengthAttribute($value): self
{
return $this->setDimension('length', $value);
}

/**
* @param mixed $value
*
* @return self
* @noinspection PhpUnused
*/
protected function setWidthAttribute($value): self
{
return $this->setDimension('width', $value);
}

/**
* Dimensions are optional and intentionally not validated: 0 and -1 are passed through to the
* API unchanged. An empty field must not become 0 though, and the admin's number input emits
* an empty string when cleared, which the int cast would silently turn into 0. Anything
* non-numeric therefore becomes null, so the key is omitted from storage and from the request.
*
* @param string $key
* @param mixed $value
*
* @return self
*/
private function setDimension(string $key, $value): self
{
$this->attributes[$key] = is_numeric($value) ? (int) $value : null;

return $this;
}
}
12 changes: 8 additions & 4 deletions src/Shipment/Model/PhysicalProperties.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@
use MyParcelNL\Pdk\Base\Model\Model;

/**
* @property null|int $height
* @property null|int $length
* @property null|int $width
* @property null|int $weight
* Dimensions are in centimeters and the weight is in grams — the units the MyParcel API expects, so
* they are sent as-is. Dimensions are optional: null means the merchant left the field empty and the
* key is omitted from the request entirely, rather than sent as 0.
*
* @property null|int $height In centimeters.
* @property null|int $length In centimeters.
* @property null|int $width In centimeters.
* @property null|int $weight In grams.
*/
class PhysicalProperties extends Model
{
Expand Down
23 changes: 22 additions & 1 deletion src/Shipment/Request/PostShipmentsRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ protected function encodeShipment(Shipment $shipment): array
'save_recipient_address' => (int) Settings::get('order.saveCustomerAddress'),
],
'options' => $this->getOptions($shipment),
'physical_properties' => ['weight' => $this->getWeight($shipment)],
'physical_properties' => $this->encodePhysicalProperties($shipment),
'pickup' => $this->getPickupLocation($shipment),
'recipient' => $this->encodeRecipient($shipment->recipient),
'reference_identifier' => $shipment->referenceIdentifier,
Expand Down Expand Up @@ -219,6 +219,27 @@ private function getPickupLocation(Shipment $shipment): ?array
]);
}

/**
* Dimensions are in centimeters, the same unit the API expects and the admin shows, so they are
Comment thread
GravendeelJochem marked this conversation as resolved.
* passed through unconverted and unvalidated. Only dimensions the merchant actually filled in
* are sent: empty ones are null and get filtered out, so an empty field never becomes a 0.
*
* @param \MyParcelNL\Pdk\Shipment\Model\Shipment $shipment
*
* @return array
*/
private function encodePhysicalProperties(Shipment $shipment): array
{
$physicalProperties = $shipment->physicalProperties;

return Utils::filterNull([
'weight' => $this->getWeight($shipment),
'length' => $physicalProperties ? $physicalProperties->length : null,
'width' => $physicalProperties ? $physicalProperties->width : null,
'height' => $physicalProperties ? $physicalProperties->height : null,
]);
Comment thread
joerivanveen marked this conversation as resolved.
}

/**
* @param \MyParcelNL\Pdk\Shipment\Model\Shipment $shipment
*
Expand Down
172 changes: 172 additions & 0 deletions tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,61 @@ function getRequestOptions(array $body, bool $orderMode, int $idx = 0): array
: ($body['data']['shipments'][$idx]['options'] ?? []);
}

/**
* Returns the physical properties array from the API request body, accounting for order vs shipment
* mode. Dimensions must reach both APIs, so every dimension test runs against both.
*/
function getRequestPhysicalProperties(array $body, bool $orderMode, int $idx = 0): array
{
return $orderMode
? ($body['data']['orders'][$idx]['shipment']['physical_properties'] ?? [])
: ($body['data']['shipments'][$idx]['physical_properties'] ?? []);
}

/**
* Stores a single order with the given physical properties, fires the export and returns the API
* request body.
*
* Concept shipments are enabled so the export stops after the create call — otherwise it goes on to
* fetch labels and shipments, and the last recorded request would no longer be the one under test.
*/
function exportWithPhysicalProperties(bool $orderMode, array $physicalProperties): array
{
TestBootstrapper::hasSubscriptionFeatures(
$orderMode ? [PdkAccountFeaturesService::FEATURE_LEGACY_ORDER_MANAGEMENT] : []
);

$fakeCarrier = factory(Carrier::class)
->withCarrier('POSTNL')
->withAllCapabilities()
->make();

factory(Settings::class)
->withOrder(factory(OrderSettings::class)->withConceptShipments(true))
->store();

$collection = factory(PdkOrderCollection::class)
->push(
factory(PdkOrder::class)
->withDeliveryOptions(factory(DeliveryOptions::class)->withCarrier($fakeCarrier))
->withPhysicalProperties($physicalProperties)
)
->store()
->make();

MockApi::enqueue(
...$orderMode
? [new ExamplePostOrdersResponse(), new ExamplePostOrderNotesResponse()]
: [new ExamplePostShipmentsResponse()]
);

Actions::execute(PdkBackendActions::EXPORT_ORDERS, [
'orderIds' => Arr::pluck($collection->toArray(), 'externalIdentifier'),
]);

return json_decode(MockApi::ensureLastRequest()->getBody()->getContents(), true);
}

/**
* In order mode all option keys are always present (disabled ones === 0). Asserts that the target
* option equals 1 and every other integer-valued option (except structural keys) equals 0.
Expand Down Expand Up @@ -1112,3 +1167,120 @@ function () {
// explicit mock API responses.
])
->with('order mode toggle');

/**
* Manual dimensions: whatever the merchant typed is forwarded verbatim and unvalidated, in
* centimeters. Both the shipment API and the legacy order v1 API must receive them, hence the order
* mode toggle on every case.
*/
it('sends only the dimensions the merchant filled in', function (bool $orderMode) {
$body = exportWithPhysicalProperties($orderMode, ['height' => '', 'length' => 40, 'width' => 20]);

$physicalProperties = getRequestPhysicalProperties($body, $orderMode);

expect($physicalProperties)
->toHaveKey('length')
->and($physicalProperties['length'])
->toBe(40)
->and($physicalProperties)
->toHaveKey('width')
->and($physicalProperties['width'])
->toBe(20)
// An emptied field must be omitted entirely, never sent as 0.
->and($physicalProperties)
->not->toHaveKey('height');
})->with('order mode toggle');

it('omits all dimensions when the merchant filled in none', function (bool $orderMode) {
$body = exportWithPhysicalProperties($orderMode, []);

$physicalProperties = getRequestPhysicalProperties($body, $orderMode);

expect($physicalProperties)
->not->toHaveKey('height')
->and($physicalProperties)
->not->toHaveKey('length')
->and($physicalProperties)
->not->toHaveKey('width')
// Weight is unconditional and must keep being sent.
->and($physicalProperties)
->toHaveKey('weight');
})->with('order mode toggle');

it('sends zero and negative dimensions unchanged', function (bool $orderMode) {
$body = exportWithPhysicalProperties($orderMode, ['height' => 0, 'length' => -1, 'width' => 20]);

$physicalProperties = getRequestPhysicalProperties($body, $orderMode);

expect($physicalProperties['height'])
->toBe(0)
->and($physicalProperties['length'])
->toBe(-1)
->and($physicalProperties['width'])
->toBe(20);
})->with('order mode toggle');

/**
* The admin form always posts a physicalProperties object, and it is empty whenever the merchant did
* not touch the dimension fields (the form serializer emits the enabled fields as undefined, which
* JSON.stringify drops). That empty object must merge to a no-op and leave stored dimensions intact.
*/
it('keeps stored dimensions when the form posts an empty physicalProperties object', function (bool $orderMode) {
TestBootstrapper::hasSubscriptionFeatures(
$orderMode ? [PdkAccountFeaturesService::FEATURE_LEGACY_ORDER_MANAGEMENT] : []
);

$fakeCarrier = factory(Carrier::class)
->withCarrier('POSTNL')
->withAllCapabilities()
->make();

factory(Settings::class)
->withOrder(factory(OrderSettings::class)->withConceptShipments(true))
->store();

$orderFactory = factory(PdkOrderCollection::class)->push(
factory(PdkOrder::class)
->withDeliveryOptions(factory(DeliveryOptions::class)->withCarrier($fakeCarrier))
->withPhysicalProperties(['height' => 30, 'length' => 40, 'width' => 20])
);

$orders = new Collection($orderFactory->make());

$orderFactory->store();

MockApi::enqueue(
...$orderMode
? [new ExamplePostOrdersResponse(), new ExamplePostOrderNotesResponse()]
: [new ExamplePostShipmentsResponse()]
);

Actions::execute(
new Request(
[
'action' => PdkBackendActions::EXPORT_ORDERS,
'orderIds' => $orders
->pluck('externalIdentifier')
->toArray(),
],
[],
[],
[],
[],
[],
json_encode(['data' => ['orders' => [['physicalProperties' => []]]]])
)
);

$physicalProperties = getRequestPhysicalProperties(
json_decode(MockApi::ensureLastRequest()->getBody()->getContents(), true),
$orderMode
);

expect($physicalProperties['height'])
->toBe(30)
->and($physicalProperties['length'])
->toBe(40)
->and($physicalProperties['width'])
->toBe(20);
})->with('order mode toggle');
64 changes: 62 additions & 2 deletions tests/Unit/App/Order/Model/PdkPhysicalPropertiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,66 @@

expect($physicalProperties->toStorableArray())->toBe($result);
})->with([
'manual weight set' => [2000, ['manualWeight' => 2000]],
'manual weight -1' => [TriStateService::INHERIT, []],
'manual weight set' => [
2000,
['height' => 30, 'length' => 40, 'width' => 20, 'manualWeight' => 2000],
],
'manual weight -1' => [
TriStateService::INHERIT,
['height' => 30, 'length' => 40, 'width' => 20],
],
]);

it('only stores dimensions the merchant filled in', function ($input, array $result) {
$physicalProperties = factory(PdkPhysicalProperties::class)
->fromScratch()
->make();

$physicalProperties->fill(['height' => $input, 'length' => $input, 'width' => $input]);

expect($physicalProperties->toStorableArray())->toBe($result);
})->with([
'empty string is omitted, not stored as 0' => [
'',
[],
],

'null is omitted' => [
null,
[],
],

'non-numeric input is omitted' => [
'abc',
[],
],

'zero is stored as-is' => [
0,
['height' => 0, 'length' => 0, 'width' => 0],
],

'negative values are stored as-is' => [
-1,
['height' => -1, 'length' => -1, 'width' => -1],
],

'numeric strings are cast to int' => [
'30',
['height' => 30, 'length' => 30, 'width' => 30],
],
]);

it('clears a previously filled dimension when the field is emptied', function () {
$physicalProperties = factory(PdkPhysicalProperties::class)
->fromScratch()
->with(['height' => 30, 'length' => 40, 'width' => 20])
->make();

$physicalProperties->fill(['height' => '']);

expect($physicalProperties->height)
->toBeNull()
->and($physicalProperties->toStorableArray())
->toBe(['length' => 40, 'width' => 20]);
});
Loading