From df5f8bb16eeffa9d0273ff7a21902680db57faa4 Mon Sep 17 00:00:00 2001 From: Joeri van Veen Date: Tue, 4 Aug 2026 15:02:12 +0200 Subject: [PATCH 1/2] feat: send width height length during export INT-1775 --- src/App/Order/Model/PdkPhysicalProperties.php | 54 ++++++++++++++++ src/Shipment/Request/PostShipmentsRequest.php | 23 ++++++- .../Order/Model/PdkPhysicalPropertiesTest.php | 64 ++++++++++++++++++- 3 files changed, 138 insertions(+), 3 deletions(-) diff --git a/src/App/Order/Model/PdkPhysicalProperties.php b/src/App/Order/Model/PdkPhysicalProperties.php index 2b487842d..adeb7d937 100644 --- a/src/App/Order/Model/PdkPhysicalProperties.php +++ b/src/App/Order/Model/PdkPhysicalProperties.php @@ -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, ]); } @@ -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; + } } diff --git a/src/Shipment/Request/PostShipmentsRequest.php b/src/Shipment/Request/PostShipmentsRequest.php index e356f96e2..3ea7d32f2 100644 --- a/src/Shipment/Request/PostShipmentsRequest.php +++ b/src/Shipment/Request/PostShipmentsRequest.php @@ -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, @@ -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 + * 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, + ]); + } + /** * @param \MyParcelNL\Pdk\Shipment\Model\Shipment $shipment * diff --git a/tests/Unit/App/Order/Model/PdkPhysicalPropertiesTest.php b/tests/Unit/App/Order/Model/PdkPhysicalPropertiesTest.php index 412d16102..eda0d0196 100644 --- a/tests/Unit/App/Order/Model/PdkPhysicalPropertiesTest.php +++ b/tests/Unit/App/Order/Model/PdkPhysicalPropertiesTest.php @@ -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]); +}); From 5a43ddb1dd94becf49e11fef8c376826a9fe3ef3 Mon Sep 17 00:00:00 2001 From: Joeri van Veen Date: Tue, 4 Aug 2026 16:54:19 +0200 Subject: [PATCH 2/2] fix: implement feedback --- src/App/Order/Model/PdkPhysicalProperties.php | 6 +- src/Shipment/Model/PhysicalProperties.php | 12 +- .../Backend/Order/ExportOrderActionTest.php | 172 ++++++++++++++++++ 3 files changed, 183 insertions(+), 7 deletions(-) diff --git a/src/App/Order/Model/PdkPhysicalProperties.php b/src/App/Order/Model/PdkPhysicalProperties.php index adeb7d937..acf6db92b 100644 --- a/src/App/Order/Model/PdkPhysicalProperties.php +++ b/src/App/Order/Model/PdkPhysicalProperties.php @@ -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 diff --git a/src/Shipment/Model/PhysicalProperties.php b/src/Shipment/Model/PhysicalProperties.php index 6db38f4e4..c6eaf44d0 100644 --- a/src/Shipment/Model/PhysicalProperties.php +++ b/src/Shipment/Model/PhysicalProperties.php @@ -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 { diff --git a/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php b/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php index 4a46f417a..e7550f5b0 100644 --- a/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php +++ b/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php @@ -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. @@ -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');