From 4e1e5452b4b3346d7bb0934b57458faafe57591c Mon Sep 17 00:00:00 2001 From: Freek van Rijt Date: Thu, 23 Jul 2026 10:55:08 +0200 Subject: [PATCH 1/4] feat(checkout): pass calculated shipment options to the checkout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checkout context now contains, per carrier, the shipment options an order from this cart would be exported with — resolved through the same settings chain and capabilities rules the export runs. The delivery options widget uses this to show and lock options that are already decided on the merchant side, like 18+ forcing signature and only recipient on. Also fixes the carrier settings screen locking signature and only recipient while 18+ was merely inheriting, and gives the test fixtures the real age verification rules from the capabilities API so the requires propagation is pinned by tests end to end. Resolves INT-1596 Co-Authored-By: Claude Fable 5 --- .../DeliveryOptionsServiceInterface.php | 9 ++ .../Service/DeliveryOptionsService.php | 92 +++++++++++- src/Context/Model/CheckoutContext.php | 53 +++++-- src/Frontend/View/CarrierSettingsItemView.php | 12 +- .../Collection/ShipmentOptionsCollection.php | 16 ++ src/Shipment/Model/ShipmentOptions.php | 34 +++++ .../MockCarrierCapabilitiesRepository.php | 9 +- .../ExampleContractDefinitionsResponse.php | 5 + .../Backend/Order/ExportOrderActionTest.php | 10 +- ...yOptionsServiceCartShipmentOptionsTest.php | 140 ++++++++++++++++++ .../Service/PdkOrderOptionsServiceTest.php | 47 +++++- .../View/CarrierSettingsItemViewTest.php | 107 +++++++++++++ ...h_data_set_delivery_options_config__1.json | 1 + ...ent_with_data_set_delivery_options__1.json | 1 + ...nent_with_data_set_plugin_settings__1.json | 96 ++++++++---- ...iew_with_data_set_carrier_settings__1.json | 96 ++++++++---- 16 files changed, 633 insertions(+), 95 deletions(-) create mode 100644 src/Shipment/Collection/ShipmentOptionsCollection.php create mode 100644 tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCartShipmentOptionsTest.php diff --git a/src/App/DeliveryOptions/Contract/DeliveryOptionsServiceInterface.php b/src/App/DeliveryOptions/Contract/DeliveryOptionsServiceInterface.php index 87d0f8193..6bb3bc3be 100644 --- a/src/App/DeliveryOptions/Contract/DeliveryOptionsServiceInterface.php +++ b/src/App/DeliveryOptions/Contract/DeliveryOptionsServiceInterface.php @@ -5,6 +5,7 @@ namespace MyParcelNL\Pdk\App\DeliveryOptions\Contract; use MyParcelNL\Pdk\App\Cart\Model\PdkCart; +use MyParcelNL\Pdk\Shipment\Collection\ShipmentOptionsCollection; interface DeliveryOptionsServiceInterface { @@ -12,4 +13,12 @@ interface DeliveryOptionsServiceInterface * Creates an array with the packageType and carrierSettings key of the delivery options config. */ public function createAllCarrierSettings(PdkCart $cart): array; + + /** + * Calculate, per carrier, the shipment options this cart would be exported with — the same + * settings chain and capabilities rules the real export runs — so the checkout can show and + * lock options that are already decided on the merchant side (for example 18+ forcing + * signature and only recipient on). The collection is keyed by legacy carrier identifier. + */ + public function createCartShipmentOptions(PdkCart $cart): ShipmentOptionsCollection; } diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index 15b56b8e6..c599fd42f 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -9,6 +9,8 @@ use MyParcelNL\Pdk\App\Cart\Contract\CartCalculationServiceInterface; use MyParcelNL\Pdk\App\Cart\Model\PdkCart; use MyParcelNL\Pdk\App\DeliveryOptions\Contract\DeliveryOptionsServiceInterface; +use MyParcelNL\Pdk\App\Order\Contract\PdkOrderOptionsServiceInterface; +use MyParcelNL\Pdk\App\Order\Model\PdkOrder; use MyParcelNL\Pdk\App\Tax\Contract\TaxServiceInterface; use MyParcelNL\Pdk\Base\Contract\CountryServiceInterface; use MyParcelNL\Pdk\Base\Contract\CurrencyServiceInterface; @@ -23,8 +25,10 @@ use MyParcelNL\Pdk\Facade\Settings; use MyParcelNL\Pdk\Settings\Model\CarrierSettings; use MyParcelNL\Pdk\Settings\Model\CheckoutSettings; +use MyParcelNL\Pdk\Shipment\Collection\ShipmentOptionsCollection; use MyParcelNL\Pdk\Shipment\Contract\DropOffServiceInterface; use MyParcelNL\Pdk\Shipment\Model\DeliveryOptions; +use MyParcelNL\Pdk\Shipment\Model\ShipmentOptions; use MyParcelNL\Sdk\Client\Generated\CoreApi\Model\RefShipmentPackageTypeV2; use MyParcelNL\Sdk\Client\Generated\CoreApi\Model\RefTypesDeliveryTypeV2; use MyParcelNL\Sdk\Support\Str; @@ -62,19 +66,25 @@ class DeliveryOptionsService implements DeliveryOptionsServiceInterface */ private $dropOffService; + /** + * @var \MyParcelNL\Pdk\App\Order\Contract\PdkOrderOptionsServiceInterface + */ + private $orderOptionsService; + /** * @var \MyParcelNL\Pdk\App\Tax\Contract\TaxServiceInterface */ private $taxService; /** - * @param \MyParcelNL\Pdk\App\Cart\Contract\CartCalculationServiceInterface $cartCalculationService - * @param \MyParcelNL\Pdk\Carrier\Service\CapabilitiesValidationService $capabilitiesValidation - * @param \MyParcelNL\Pdk\Carrier\Contract\CarrierRepositoryInterface $carrierRepository - * @param \MyParcelNL\Pdk\Base\Contract\CountryServiceInterface $countryService - * @param \MyParcelNL\Pdk\Base\Contract\CurrencyServiceInterface $currencyService - * @param \MyParcelNL\Pdk\Shipment\Contract\DropOffServiceInterface $dropOffService - * @param \MyParcelNL\Pdk\App\Tax\Contract\TaxServiceInterface $taxService + * @param \MyParcelNL\Pdk\App\Cart\Contract\CartCalculationServiceInterface $cartCalculationService + * @param \MyParcelNL\Pdk\Carrier\Service\CapabilitiesValidationService $capabilitiesValidation + * @param \MyParcelNL\Pdk\Carrier\Contract\CarrierRepositoryInterface $carrierRepository + * @param \MyParcelNL\Pdk\Base\Contract\CountryServiceInterface $countryService + * @param \MyParcelNL\Pdk\Base\Contract\CurrencyServiceInterface $currencyService + * @param \MyParcelNL\Pdk\Shipment\Contract\DropOffServiceInterface $dropOffService + * @param \MyParcelNL\Pdk\App\Order\Contract\PdkOrderOptionsServiceInterface $orderOptionsService + * @param \MyParcelNL\Pdk\App\Tax\Contract\TaxServiceInterface $taxService */ public function __construct( CartCalculationServiceInterface $cartCalculationService, @@ -83,6 +93,7 @@ public function __construct( CountryServiceInterface $countryService, CurrencyServiceInterface $currencyService, DropOffServiceInterface $dropOffService, + PdkOrderOptionsServiceInterface $orderOptionsService, TaxServiceInterface $taxService ) { $this->cartCalculationService = $cartCalculationService; @@ -91,6 +102,7 @@ public function __construct( $this->countryService = $countryService; $this->currencyService = $currencyService; $this->dropOffService = $dropOffService; + $this->orderOptionsService = $orderOptionsService; $this->taxService = $taxService; } @@ -133,6 +145,72 @@ public function createAllCarrierSettings(PdkCart $cart): array return $settings; } + /** + * Calculate, per carrier, the shipment options this cart would be exported with, so the + * checkout can show and lock options that are already decided on the merchant side. Runs + * the same full calculation pipeline as an export (settings chain plus capabilities + * requires/excludes rules) on an order built from the cart, and converts the result to + * the capabilities key-space through the same converter the admin context uses. + * + * @param \MyParcelNL\Pdk\App\Cart\Model\PdkCart $cart + * + * @return \MyParcelNL\Pdk\Shipment\Collection\ShipmentOptionsCollection keyed by legacy + * carrier identifier. + */ + public function createCartShipmentOptions(PdkCart $cart): ShipmentOptionsCollection + { + if (! $cart->shippingMethod->hasDeliveryOptions) { + return new ShipmentOptionsCollection(); + } + + [$packageType, $carriers] = $this->getValidCarrierOptions($cart); + + $cartShipmentOptions = new ShipmentOptionsCollection(); + + foreach ($carriers as $carrier) { + if (null === $carrier->carrier) { + continue; + } + + // Use the legacy identifier, matching the carrierSettings keys in the config. + $identifier = FrontendData::getLegacyCarrierIdentifier($carrier->carrier); + + $cartShipmentOptions->put($identifier, $this->calculateCartShipmentOptions($carrier, $cart, $packageType)); + } + + return $cartShipmentOptions; + } + + /** + * Build an order representing what this cart would ship with for the given carrier, run + * the full calculation pipeline on it, and return its calculated shipment options. + * Mirrors the per-carrier synthetic order the admin context uses for inherited delivery + * options ({@see \MyParcelNL\Pdk\Context\Model\OrderDataContext}). + * + * @param \MyParcelNL\Pdk\Carrier\Model\Carrier $carrier Carrier to calculate for. + * @param \MyParcelNL\Pdk\App\Cart\Model\PdkCart $cart Cart supplying the order + * lines (product settings, + * weight) and shipping address. + * @param string $packageType Package type name resolved + * for this cart, e.g. 'package'. + * + * @return \MyParcelNL\Pdk\Shipment\Model\ShipmentOptions + */ + private function calculateCartShipmentOptions(Carrier $carrier, PdkCart $cart, string $packageType): ShipmentOptions + { + $order = new PdkOrder([ + 'lines' => $cart->lines, + 'shippingAddress' => $cart->shippingMethod->shippingAddress, + 'deliveryOptions' => ['packageType' => $packageType], + ]); + + $order->deliveryOptions->carrier = $carrier; + + $calculatedOrder = $this->orderOptionsService->calculate($order); + + return $calculatedOrder->deliveryOptions->shipmentOptions; + } + /** * Create the settings for a specific carrier based on the cart. * @param \MyParcelNL\Pdk\Carrier\Model\Carrier $carrier diff --git a/src/Context/Model/CheckoutContext.php b/src/Context/Model/CheckoutContext.php index 4028f806e..7fdf8e40c 100644 --- a/src/Context/Model/CheckoutContext.php +++ b/src/Context/Model/CheckoutContext.php @@ -6,7 +6,7 @@ use MyParcelNL\Pdk\App\Api\Contract\FrontendEndpointServiceInterface; use MyParcelNL\Pdk\App\Cart\Model\PdkCart; -use MyParcelNL\Pdk\App\DeliveryOptions\Service\DeliveryOptionsService; +use MyParcelNL\Pdk\App\DeliveryOptions\Contract\DeliveryOptionsServiceInterface; use MyParcelNL\Pdk\App\Request\Collection\EndpointRequestCollection; use MyParcelNL\Pdk\Base\Model\Model; use MyParcelNL\Pdk\Base\Support\Collection; @@ -15,27 +15,37 @@ use MyParcelNL\Pdk\Facade\Pdk; use MyParcelNL\Pdk\Facade\Settings; use MyParcelNL\Pdk\Settings\Model\CheckoutSettings; +use MyParcelNL\Pdk\Shipment\Model\ShipmentOptions; use MyParcelNL\Sdk\Support\Str; /** - * @property null|DeliveryOptionsConfig $config - * @property array{string,string} $strings - * @property array $settings + * @property null|DeliveryOptionsConfig $config + * @property Collection> $cartShipmentOptions + * @property array{string,string} $strings + * @property array $settings */ class CheckoutContext extends Model { public $attributes = [ - 'config' => null, - 'strings' => [], - 'settings' => [], - 'endpoints' => EndpointRequestCollection::class, + 'config' => null, + /** + * The shipment options this cart would be exported with, calculated per carrier — + * cart state, deliberately next to `config` rather than inside it. The checkout + * widget uses it to show and lock options that are already decided on the merchant + * side (e.g. 18+ forcing signature and only recipient on). + */ + 'cartShipmentOptions' => Collection::class, + 'strings' => [], + 'settings' => [], + 'endpoints' => EndpointRequestCollection::class, ]; protected $casts = [ - 'config' => DeliveryOptionsConfig::class, - 'strings' => 'array', - 'settings' => 'array', - 'endpoints' => EndpointRequestCollection::class, + 'config' => DeliveryOptionsConfig::class, + 'cartShipmentOptions' => Collection::class, + 'strings' => 'array', + 'settings' => 'array', + 'endpoints' => EndpointRequestCollection::class, ]; /** @@ -56,10 +66,23 @@ public function __construct(?array $data = null) */ public static function fromCart(PdkCart $cart): self { - return new self([ + /** @var \MyParcelNL\Pdk\App\DeliveryOptions\Contract\DeliveryOptionsServiceInterface $deliveryOptionsService */ + $deliveryOptionsService = Pdk::get(DeliveryOptionsServiceInterface::class); + + // The widget represents shipment options as booleans (its DeliveryOptionsOutput + // format), so the calculated models are converted at this wire boundary. Built as a + // plain array on purpose: mapping on the typed collection would cast the boolean + // arrays straight back into ShipmentOptions models. + $cartShipmentOptions = []; - 'config' => DeliveryOptionsConfig::fromCart($cart), - 'settings' => [ + foreach ($deliveryOptionsService->createCartShipmentOptions($cart)->all() as $identifier => $shipmentOptions) { + $cartShipmentOptions[$identifier] = ShipmentOptions::toBooleanOptions($shipmentOptions); + } + + return new self([ + 'config' => DeliveryOptionsConfig::fromCart($cart), + 'cartShipmentOptions' => $cartShipmentOptions, + 'settings' => [ 'hasDeliveryOptions' => $cart->shippingMethod->hasDeliveryOptions, ], ]); diff --git a/src/Frontend/View/CarrierSettingsItemView.php b/src/Frontend/View/CarrierSettingsItemView.php index 08fa42d1e..f542bc9ff 100644 --- a/src/Frontend/View/CarrierSettingsItemView.php +++ b/src/Frontend/View/CarrierSettingsItemView.php @@ -268,8 +268,11 @@ private function getDefaultExportFields(): array $ageCheckElement = (new InteractiveElement($ageCheckDefinition->getCarrierSettingsKey(), Components::INPUT_TOGGLE)) ->builder(function (FormOperationBuilder $builder) use ($signatureKey, $onlyRecipientKey) { $builder->afterUpdate(function (FormAfterUpdateBuilder $afterUpdate) use ($signatureKey, $onlyRecipientKey) { - // Toggle settings are stored as TriState integers (-1/0/1); operands - // must match so the frontend's strict `$eq` check succeeds. + // The admin form normalizes toggle values to TriState ints (1/0/-1) + // at the component boundary, so the strict `$eq` checks below compare + // int against int. Turning age check OFF deliberately leaves signature + // and only recipient stored as enabled — they just unlock again, so + // what the merchant sees is exactly what is stored. if ($signatureKey) { $afterUpdate->setValue(TriStateService::ENABLED)->on($signatureKey)->if->eq(TriStateService::ENABLED); } @@ -303,7 +306,10 @@ function (FormOperationBuilder $builder) use ($ageCheckDefinition) { if (! $ageCheckDefinition || ! $this->carrierValidationService->supportsShipmentOption($this->carrier, $ageCheckDefinition)) { return; } - $builder->readOnlyWhen($ageCheckDefinition->getCarrierSettingsKey()); + // Lock only while age check is explicitly enabled. A bare (truthy) check + // would also match INHERIT (-1) and lock the fields while age check was + // never turned on. + $builder->readOnlyWhen($ageCheckDefinition->getCarrierSettingsKey(), TriStateService::ENABLED); }, $signatureElements, $onlyRecipientElements diff --git a/src/Shipment/Collection/ShipmentOptionsCollection.php b/src/Shipment/Collection/ShipmentOptionsCollection.php new file mode 100644 index 000000000..aec451204 --- /dev/null +++ b/src/Shipment/Collection/ShipmentOptionsCollection.php @@ -0,0 +1,16 @@ + true, disabled => false). Unset (inherit) + * options and non-boolean attributes such as the insurance amount and label description + * are left out. + * + * @param ShipmentOptions $shipmentOptions + * + * @return array keyed by shipment option key (e.g. `onlyRecipient`) + */ + public static function toBooleanOptions(ShipmentOptions $shipmentOptions): array + { + $data = []; + + /** @var OrderOptionDefinitionInterface[] $definitions */ + $definitions = Pdk::get('orderOptionDefinitions'); + + foreach ($definitions as $definition) { + $shipmentOptionsKey = $definition->getShipmentOptionsKey(); + + if (! $shipmentOptionsKey || TriStateService::TYPE_STRICT !== $definition->getShipmentOptionsCast()) { + continue; + } + + $value = $shipmentOptions->getAttribute($shipmentOptionsKey); + + if (TriStateService::ENABLED === $value || TriStateService::DISABLED === $value) { + $data[$shipmentOptionsKey] = TriStateService::ENABLED === $value; + } + } + + return $data; + } } diff --git a/tests/Bootstrap/MockCarrierCapabilitiesRepository.php b/tests/Bootstrap/MockCarrierCapabilitiesRepository.php index 4bad4cdc6..c11267a21 100644 --- a/tests/Bootstrap/MockCarrierCapabilitiesRepository.php +++ b/tests/Bootstrap/MockCarrierCapabilitiesRepository.php @@ -107,7 +107,14 @@ private function buildPermissiveCapabilities(string $carrierName): array 'options' => [ 'requiresSignature' => $option, 'recipientOnlyDelivery' => $option, - 'requiresAgeVerification' => $option, + // Age verification carries its real relations (verified against the live + // capabilities API): it requires signature + only recipient and excludes + // receipt code. `printReturnLabelAtDropOff` has no PDK option definition and + // is dropped by the calculator — realistic, the live data contains it too. + 'requiresAgeVerification' => array_merge($option, [ + 'requires' => ['recipientOnlyDelivery', 'requiresSignature'], + 'excludes' => ['printReturnLabelAtDropOff', 'requiresReceiptCode'], + ]), 'oversizedPackage' => $option, 'hideSender' => $option, 'returnOnFirstFailedDelivery' => $option, diff --git a/tests/SdkApi/Response/ExampleContractDefinitionsResponse.php b/tests/SdkApi/Response/ExampleContractDefinitionsResponse.php index 02cf7bcbe..b3558b5a0 100644 --- a/tests/SdkApi/Response/ExampleContractDefinitionsResponse.php +++ b/tests/SdkApi/Response/ExampleContractDefinitionsResponse.php @@ -26,6 +26,11 @@ * Pass a custom $items array to the constructor to override defaults for a specific test: * new ExampleContractDefinitionsResponse([['carrier' => 'POSTNL', ...]]); * + * Note: contract-definitions options carry ONLY isSelectedByDefault and isRequired. The + * requires/excludes relations between options exist exclusively on the live capabilities + * response (RefCapabilitiesResponseOptionsOptionV2) — that asymmetry mirrors the real API, + * so do not add requires/excludes here. + * * @see \MyParcelNL\Sdk\Client\Generated\CoreApi\Model\CapabilitiesResponsesContractDefinitionsV2 * @see \MyParcelNL\Sdk\Client\Generated\CoreApi\Model\RefCapabilitiesContractDefinitionsResponseContractDefinitionsV2 * @see \MyParcelNL\Sdk\Client\Generated\CoreApi\Model\CapabilitiesPostContractDefinitionsRequestV2 diff --git a/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php b/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php index e7550f5b0..58c82620c 100644 --- a/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php +++ b/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php @@ -294,10 +294,12 @@ function exportWithSetting(bool $orderMode, CarrierSettingsFactory $carrierSetti $body = exportWithSetting($orderMode, factory(CarrierSettings::class)->withExportAgeCheck(true)); $options = getRequestOptions($body, $orderMode); - // Age check is enabled via export setting. - // Cascade behavior (age_check requires signature + only_recipient) is now driven - // by the capabilities API requires field, tested in CapabilitiesOptionCalculatorTest. - expect($options['age_check'])->toBe(1); + // Age check is enabled via the export setting. The capabilities requires/excludes + // relations (the permissive mock carries the real age verification rules) force + // signature and only recipient on with it, and receipt code off. + expect($options['age_check'])->toBe(1) + ->and($options['signature'])->toBe(1) + ->and($options['only_recipient'])->toBe(1); }) ->with('order mode toggle'); diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCartShipmentOptionsTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCartShipmentOptionsTest.php new file mode 100644 index 000000000..ebf3668ab --- /dev/null +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCartShipmentOptionsTest.php @@ -0,0 +1,140 @@ +group('checkout'); + +usesShared(new UsesMockPdkInstance(), new UsesAccountMock()); + +/** + * A deliverable single-line cart with an NL shipping address — the minimum a capabilities + * lookup needs (country + weight). + */ +function cartWithNlAddress(array $productSettings = []): PdkCart +{ + return new PdkCart([ + 'shippingMethod' => [ + 'shippingAddress' => ['cc' => 'NL'], + ], + 'lines' => [ + [ + 'quantity' => 1, + 'product' => [ + 'isDeliverable' => true, + 'weight' => 1, + 'settings' => $productSettings, + ], + ], + ], + ]); +} + +function storeShopWithCarrier(string $carrierName, callable $settingsCallback = null): void +{ + $carrierFactory = factory(Carrier::class) + ->withCarrier($carrierName) + ->withAllCapabilities(); + + $settingsFactory = factory(CarrierSettings::class, $carrierName)->withDeliveryOptions(); + + if ($settingsCallback) { + $settingsFactory = $settingsCallback($settingsFactory); + } + + $settingsFactory->store(); + + factory(Shop::class) + ->withCarriers(factory(CarrierCollection::class)->push($carrierFactory)) + ->store(); +} + +it('calculates shipment options per carrier including capability requires', function () { + storeShopWithCarrier('POSTNL', function (object $settings) { + return $settings->withExportAgeCheck(true); + }); + + /** @var \MyParcelNL\Pdk\App\DeliveryOptions\Contract\DeliveryOptionsServiceInterface $service */ + $service = Pdk::get(DeliveryOptionsServiceInterface::class); + + $result = $service->createCartShipmentOptions(cartWithNlAddress()); + + /** @var ShipmentOptions $options */ + $options = $result->get('postnl'); + + expect($options)->toBeInstanceOf(ShipmentOptions::class) + ->and($options->ageCheck)->toBe(TriStateService::ENABLED) + ->and($options->signature)->toBe(TriStateService::ENABLED) + ->and($options->onlyRecipient)->toBe(TriStateService::ENABLED) + ->and($options->receiptCode)->toBe(TriStateService::DISABLED); +}); + +it('includes options activated through product settings', function () { + storeShopWithCarrier('POSTNL'); + + /** @var \MyParcelNL\Pdk\App\DeliveryOptions\Contract\DeliveryOptionsServiceInterface $service */ + $service = Pdk::get(DeliveryOptionsServiceInterface::class); + + $result = $service->createCartShipmentOptions( + cartWithNlAddress(['exportAgeCheck' => TriStateService::ENABLED]) + ); + + /** @var ShipmentOptions $options */ + $options = $result->get('postnl'); + + expect($options->ageCheck)->toBe(TriStateService::ENABLED) + ->and($options->signature)->toBe(TriStateService::ENABLED) + ->and($options->onlyRecipient)->toBe(TriStateService::ENABLED); +}); + +it('exposes the options as widget-format booleans through the checkout context', function () { + storeShopWithCarrier('POSTNL', function (object $settings) { + return $settings->withExportAgeCheck(true); + }); + + $context = CheckoutContext::fromCart(cartWithNlAddress()); + + expect($context->cartShipmentOptions)->toHaveKey('postnl') + ->and($context->cartShipmentOptions['postnl']['ageCheck'])->toBe(true) + ->and($context->cartShipmentOptions['postnl']['signature'])->toBe(true) + ->and($context->cartShipmentOptions['postnl']['onlyRecipient'])->toBe(true) + ->and($context->cartShipmentOptions['postnl']['receiptCode'])->toBe(false) + ->and($context->toArray())->toHaveKey('cartShipmentOptions'); +}); + +it('always contains an entry per carrier with definitive values', function () { + storeShopWithCarrier('POSTNL'); + + /** @var \MyParcelNL\Pdk\App\DeliveryOptions\Contract\DeliveryOptionsServiceInterface $service */ + $service = Pdk::get(DeliveryOptionsServiceInterface::class); + + $result = $service->createCartShipmentOptions(cartWithNlAddress()); + + /** @var ShipmentOptions $options */ + $options = $result->get('postnl'); + + // The calculation pipeline always resolves options to definitive values (that is what an + // export needs), so inactive options come out as explicit DISABLED — never as INHERIT. + expect($options)->toBeInstanceOf(ShipmentOptions::class) + ->and($options->ageCheck)->toBe(TriStateService::DISABLED) + ->and($options->signature)->toBe(TriStateService::DISABLED); +}); diff --git a/tests/Unit/App/Order/Service/PdkOrderOptionsServiceTest.php b/tests/Unit/App/Order/Service/PdkOrderOptionsServiceTest.php index 417a4d72c..b1c54cdc4 100644 --- a/tests/Unit/App/Order/Service/PdkOrderOptionsServiceTest.php +++ b/tests/Unit/App/Order/Service/PdkOrderOptionsServiceTest.php @@ -6,12 +6,12 @@ namespace MyParcelNL\Pdk\App\Order\Service; +use MyParcelNL\Pdk\App\Options\Definition\AgeCheckDefinition; use MyParcelNL\Pdk\App\Options\Definition\SignatureDefinition; use MyParcelNL\Pdk\App\Order\Contract\PdkOrderOptionsServiceInterface; use MyParcelNL\Pdk\App\Order\Model\PdkOrder; use MyParcelNL\Pdk\Carrier\Model\Carrier; use MyParcelNL\Pdk\Facade\Pdk; -use MyParcelNL\Pdk\Settings\Model\CarrierSettings; use MyParcelNL\Pdk\Settings\Model\Settings; use MyParcelNL\Pdk\Shipment\Model\DeliveryOptions; use MyParcelNL\Pdk\Shipment\Model\ShipmentOptions; @@ -216,6 +216,51 @@ expect($newOrder->deliveryOptions->shipmentOptions->signature)->toBe(TriStateService::ENABLED); }); +it('forces signature and only recipient through capabilities requires when age check comes from carrier settings', function () { + factory(Carrier::class) + ->withAllCapabilities() + ->store(); + + $storage = Pdk::get(StorageInterface::class); + $storage->delete('carrier:POSTNL'); + $storage->delete('carrier:all'); + + factory(Settings::class) + ->withCarrier('POSTNL', [(new AgeCheckDefinition())->getCarrierSettingsKey() => TriStateService::ENABLED]) + ->store(); + + // Mirror an order placed through the checkout: the consumer left signature and only + // recipient unchecked (explicit DISABLED); age check is merchant-only and unset on the + // order, so it comes in through the carrier settings. + $order = factory(PdkOrder::class) + ->withShippingAddress(['cc' => 'NL']) + ->withDeliveryOptions( + factory(DeliveryOptions::class) + ->withCarrier('POSTNL') + ->withPackageType('package') + ->withDeliveryType('standard') + ->withShipmentOptions( + factory(ShipmentOptions::class) + ->withAgeCheck(TriStateService::INHERIT) + ->withSignature(TriStateService::DISABLED) + ->withOnlyRecipient(TriStateService::DISABLED) + ->withReceiptCode(TriStateService::INHERIT) + ) + ) + ->make(); + + /** @var PdkOrderOptionsServiceInterface $service */ + $service = Pdk::get(PdkOrderOptionsServiceInterface::class); + $newOrder = $service->calculate($order); + + $shipmentOptions = $newOrder->deliveryOptions->shipmentOptions; + + expect($shipmentOptions->ageCheck)->toBe(TriStateService::ENABLED) + ->and($shipmentOptions->signature)->toBe(TriStateService::ENABLED) + ->and($shipmentOptions->onlyRecipient)->toBe(TriStateService::ENABLED) + ->and($shipmentOptions->receiptCode)->toBe(TriStateService::DISABLED); +}); + it('isRequired resolves to ENABLED via capabilities default when carrier setting is INHERIT', function () { factory(Carrier::class) ->withAllCapabilities() diff --git a/tests/Unit/Frontend/View/CarrierSettingsItemViewTest.php b/tests/Unit/Frontend/View/CarrierSettingsItemViewTest.php index afc77832b..c49924a07 100644 --- a/tests/Unit/Frontend/View/CarrierSettingsItemViewTest.php +++ b/tests/Unit/Frontend/View/CarrierSettingsItemViewTest.php @@ -424,3 +424,110 @@ function () { expect($deliveryOptionsFound)->toBeTrue(); }); + +/** + * Find a form element by name in a view's array representation. + */ +function findCarrierSettingsElement(array $viewArray, string $name): ?array +{ + foreach ($viewArray['elements'] as $element) { + if (($element['name'] ?? null) === $name) { + return $element; + } + } + + return null; +} + +it('emits strict tri-state operations on the age check toggle', function () { + $carrier = factory(Carrier::class) + ->withAllCapabilities() + ->store() + ->make(); + + $view = new CarrierSettingsItemView($carrier); + $element = findCarrierSettingsElement($view->toArray(Arrayable::SKIP_NULL), 'exportAgeCheck'); + + expect($element)->not->toBeNull(); + + $afterUpdate = null; + + foreach ($element['$builders'] ?? [] as $builder) { + if (isset($builder['$afterUpdate'])) { + $afterUpdate = $builder['$afterUpdate']; + } + } + + expect($afterUpdate)->toEqual([ + [ + '$setValue' => [ + '$value' => \MyParcelNL\Pdk\Types\Service\TriStateService::ENABLED, + '$target' => 'exportSignature', + '$if' => [['$eq' => \MyParcelNL\Pdk\Types\Service\TriStateService::ENABLED]], + ], + ], + [ + '$setValue' => [ + '$value' => \MyParcelNL\Pdk\Types\Service\TriStateService::ENABLED, + '$target' => 'exportOnlyRecipient', + '$if' => [['$eq' => \MyParcelNL\Pdk\Types\Service\TriStateService::ENABLED]], + ], + ], + ]) + // Guard against boolean operands sneaking back in: `toEqual` would accept true == 1. + ->and($afterUpdate[0]['$setValue']['$value'])->toBeInt() + ->and($afterUpdate[0]['$setValue']['$if'][0]['$eq'])->toBeInt(); +}); + +it('locks signature and only recipient only while age check is explicitly enabled', function () { + $carrier = factory(Carrier::class) + ->withAllCapabilities() + ->store() + ->make(); + + $view = new CarrierSettingsItemView($carrier); + $viewArray = $view->toArray(Arrayable::SKIP_NULL); + + foreach (['exportSignature', 'exportOnlyRecipient'] as $name) { + $element = findCarrierSettingsElement($viewArray, $name); + + expect($element)->not->toBeNull(); + + $readOnlyWhen = null; + + foreach ($element['$builders'] ?? [] as $builder) { + if (isset($builder['$readOnlyWhen'])) { + $readOnlyWhen = $builder['$readOnlyWhen']; + } + } + + expect($readOnlyWhen)->toEqual([ + '$if' => [['$target' => 'exportAgeCheck', '$eq' => \MyParcelNL\Pdk\Types\Service\TriStateService::ENABLED]], + ]) + ->and($readOnlyWhen['$if'][0]['$eq'])->toBeInt(); + } +}); + +it('emits no age check rules when the carrier lacks age verification', function () { + $carrier = factory(Carrier::class) + ->withMinimalCapabilities() + ->withCapabilityShipmentOptions([ + 'requiresSignature' => ['isSelectedByDefault' => false, 'isRequired' => false], + 'recipientOnlyDelivery' => ['isSelectedByDefault' => false, 'isRequired' => false], + ]) + ->store() + ->make(); + + $view = new CarrierSettingsItemView($carrier); + $viewArray = $view->toArray(Arrayable::SKIP_NULL); + + expect(findCarrierSettingsElement($viewArray, 'exportAgeCheck'))->toBeNull(); + + $signatureElement = findCarrierSettingsElement($viewArray, 'exportSignature'); + + expect($signatureElement)->not->toBeNull(); + + foreach ($signatureElement['$builders'] ?? [] as $builder) { + expect($builder['$readOnlyWhen']['$if'][0]['$target'] ?? null)->not->toBe('exportAgeCheck'); + } +}); diff --git a/tests/__snapshots__/ContextServiceTest__it_gets_context_data_with_data_set_delivery_options_config__1.json b/tests/__snapshots__/ContextServiceTest__it_gets_context_data_with_data_set_delivery_options_config__1.json index d85354db3..f3ba8f5fb 100644 --- a/tests/__snapshots__/ContextServiceTest__it_gets_context_data_with_data_set_delivery_options_config__1.json +++ b/tests/__snapshots__/ContextServiceTest__it_gets_context_data_with_data_set_delivery_options_config__1.json @@ -1,5 +1,6 @@ { "checkout": { + "cartShipmentOptions": [], "strings": { "morning": "Ochtend" }, diff --git a/tests/__snapshots__/FrontendRenderServiceTest__it_renders_component_with_data_set_delivery_options__1.json b/tests/__snapshots__/FrontendRenderServiceTest__it_renders_component_with_data_set_delivery_options__1.json index 8b37dc18a..2a8285df4 100644 --- a/tests/__snapshots__/FrontendRenderServiceTest__it_renders_component_with_data_set_delivery_options__1.json +++ b/tests/__snapshots__/FrontendRenderServiceTest__it_renders_component_with_data_set_delivery_options__1.json @@ -17,6 +17,7 @@ "isBusiness": false, "apiBaseUrl": "https:\/\/api.myparcel.nl" }, + "cartShipmentOptions": [], "strings": { "morning": "Ochtend" }, diff --git a/tests/__snapshots__/FrontendRenderServiceTest__it_renders_component_with_data_set_plugin_settings__1.json b/tests/__snapshots__/FrontendRenderServiceTest__it_renders_component_with_data_set_plugin_settings__1.json index 97343e112..f983502ec 100644 --- a/tests/__snapshots__/FrontendRenderServiceTest__it_renders_component_with_data_set_plugin_settings__1.json +++ b/tests/__snapshots__/FrontendRenderServiceTest__it_renders_component_with_data_set_plugin_settings__1.json @@ -1805,7 +1805,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -1822,7 +1823,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -3512,7 +3514,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -3529,7 +3532,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -5219,7 +5223,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -5236,7 +5241,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -6926,7 +6932,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -6943,7 +6950,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -8633,7 +8641,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -8650,7 +8659,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -10340,7 +10350,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -10357,7 +10368,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -12047,7 +12059,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -12064,7 +12077,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -13754,7 +13768,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -13771,7 +13786,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -15461,7 +15477,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -15478,7 +15495,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -17168,7 +17186,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -17185,7 +17204,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -18875,7 +18895,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -18892,7 +18913,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -20633,7 +20655,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -20650,7 +20673,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -22340,7 +22364,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -22357,7 +22382,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -24047,7 +24073,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -24064,7 +24091,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -25754,7 +25782,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -25771,7 +25800,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -27461,7 +27491,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -27478,7 +27509,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } diff --git a/tests/__snapshots__/SettingsViewTest__it_gets_settings_view_with_data_set_carrier_settings__1.json b/tests/__snapshots__/SettingsViewTest__it_gets_settings_view_with_data_set_carrier_settings__1.json index cd06e107b..a140bebbb 100644 --- a/tests/__snapshots__/SettingsViewTest__it_gets_settings_view_with_data_set_carrier_settings__1.json +++ b/tests/__snapshots__/SettingsViewTest__it_gets_settings_view_with_data_set_carrier_settings__1.json @@ -58,7 +58,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -75,7 +76,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -1765,7 +1767,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -1782,7 +1785,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -3472,7 +3476,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -3489,7 +3494,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -5179,7 +5185,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -5196,7 +5203,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -6886,7 +6894,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -6903,7 +6912,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -8593,7 +8603,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -8610,7 +8621,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -10300,7 +10312,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -10317,7 +10330,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -12007,7 +12021,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -12024,7 +12039,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -13714,7 +13730,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -13731,7 +13748,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -15421,7 +15439,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -15438,7 +15457,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -17128,7 +17148,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -17145,7 +17166,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -18886,7 +18908,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -18903,7 +18926,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -20593,7 +20617,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -20610,7 +20635,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -22300,7 +22326,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -22317,7 +22344,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -24007,7 +24035,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -24024,7 +24053,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -25714,7 +25744,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } @@ -25731,7 +25762,8 @@ "$readOnlyWhen": { "$if": [ { - "$target": "exportAgeCheck" + "$target": "exportAgeCheck", + "$eq": 1 } ] } From b014b01e733321211482e06194e3c7c0fef8a761 Mon Sep 17 00:00:00 2001 From: Freek van Rijt Date: Fri, 24 Jul 2026 11:12:06 +0200 Subject: [PATCH 2/4] fix(orders): correct cart shipment options docs and pin receipt code in the age check export test The createCartShipmentOptions docblock claimed the result is converted to the capabilities key-space; it returns calculated ShipmentOptions models, and the widget-format boolean conversion happens at the CheckoutContext boundary. The age check export test now also asserts that receipt code is off, matching the capability rules it already describes. Resolves INT-1596 Co-Authored-By: Claude Fable 5 --- src/App/DeliveryOptions/Service/DeliveryOptionsService.php | 7 +++++-- .../App/Action/Backend/Order/ExportOrderActionTest.php | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index c599fd42f..1c7d70241 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -149,8 +149,11 @@ public function createAllCarrierSettings(PdkCart $cart): array * Calculate, per carrier, the shipment options this cart would be exported with, so the * checkout can show and lock options that are already decided on the merchant side. Runs * the same full calculation pipeline as an export (settings chain plus capabilities - * requires/excludes rules) on an order built from the cart, and converts the result to - * the capabilities key-space through the same converter the admin context uses. + * requires/excludes rules) on an order built from the cart. + * + * The result holds calculated ShipmentOptions models with tri-state values; converting + * them to the widget's boolean format happens at the CheckoutContext boundary + * ({@see \MyParcelNL\Pdk\Shipment\Model\ShipmentOptions::toBooleanOptions()}). * * @param \MyParcelNL\Pdk\App\Cart\Model\PdkCart $cart * diff --git a/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php b/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php index 58c82620c..fdf764bfa 100644 --- a/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php +++ b/tests/Unit/App/Action/Backend/Order/ExportOrderActionTest.php @@ -299,7 +299,8 @@ function exportWithSetting(bool $orderMode, CarrierSettingsFactory $carrierSetti // signature and only recipient on with it, and receipt code off. expect($options['age_check'])->toBe(1) ->and($options['signature'])->toBe(1) - ->and($options['only_recipient'])->toBe(1); + ->and($options['only_recipient'])->toBe(1) + ->and($options['receipt_code'] ?? 0)->toBe(0); }) ->with('order mode toggle'); From c88cb064ae42ba77fa72f392232f8f7f899fe8ad Mon Sep 17 00:00:00 2001 From: Freek van Rijt Date: Tue, 28 Jul 2026 16:18:00 +0200 Subject: [PATCH 3/4] fix(checkout): stop the checkout from breaking when the label description uses the customer note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building the checkout context runs the cart through the same calculation an export does, on an order that only exists in memory. With [CUSTOMER_NOTE] in the label description setting, that calculation asked the shop for the order's notes, and looking notes up without an order identifier throws in WooCommerce and PrestaShop — taking the checkout down with it. An order without an identifier now simply has no notes, so nothing asks the shop for them, and the cart's order is built with empty notes to say so up front. Tests use a note repository that rejects a missing identifier like the platforms do, so this cannot pass silently again. Resolves INT-1596 Co-Authored-By: Claude Fable 5 --- .../Service/DeliveryOptionsService.php | 3 ++ src/App/Order/Model/PdkOrder.php | 7 ++++ .../MockStrictPdkOrderNoteRepository.php | 39 +++++++++++++++++++ ...yOptionsServiceCartShipmentOptionsTest.php | 31 ++++++++++++++- tests/Unit/App/Order/Model/PdkOrderTest.php | 20 ++++++++++ 5 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/Bootstrap/MockStrictPdkOrderNoteRepository.php diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index 1c7d70241..bebd8cf57 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -205,6 +205,9 @@ private function calculateCartShipmentOptions(Carrier $carrier, PdkCart $cart, s 'lines' => $cart->lines, 'shippingAddress' => $cart->shippingMethod->shippingAddress, 'deliveryOptions' => ['packageType' => $packageType], + // A cart has no order notes yet, and calculators may read them (the label + // description can include the customer note). + 'notes' => [], ]); $order->deliveryOptions->carrier = $carrier; diff --git a/src/App/Order/Model/PdkOrder.php b/src/App/Order/Model/PdkOrder.php index 6e4a74ae7..bec92dc4c 100644 --- a/src/App/Order/Model/PdkOrder.php +++ b/src/App/Order/Model/PdkOrder.php @@ -220,6 +220,13 @@ public function getNotesAttribute(): PdkOrderNoteCollection return $this->getCastAttributeValue('notes'); } + // Notes belong to the order in the shop, looked up by its identifier. Orders that only + // exist in memory (like the one built from a cart) have none, and asking the shop for + // the notes of a null identifier throws — which broke the checkout once. + if (null === $this->externalIdentifier) { + return new PdkOrderNoteCollection(); + } + /** @var \MyParcelNL\Pdk\App\Order\Contract\PdkOrderNoteRepositoryInterface $orderNoteRepository */ $orderNoteRepository = Pdk::get(PdkOrderNoteRepositoryInterface::class); diff --git a/tests/Bootstrap/MockStrictPdkOrderNoteRepository.php b/tests/Bootstrap/MockStrictPdkOrderNoteRepository.php new file mode 100644 index 000000000..db0ca1496 --- /dev/null +++ b/tests/Bootstrap/MockStrictPdkOrderNoteRepository.php @@ -0,0 +1,39 @@ +externalIdentifier) { + throw new InvalidArgumentException('Invalid input'); + } + + return new PdkOrderNoteCollection(); + } + + public function update(PdkOrderNote $note): void + { + // Not needed for these tests. + } +} diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCartShipmentOptionsTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCartShipmentOptionsTest.php index ebf3668ab..a8dfbabd9 100644 --- a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCartShipmentOptionsTest.php +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCartShipmentOptionsTest.php @@ -9,22 +9,34 @@ use MyParcelNL\Pdk\Account\Model\Shop; use MyParcelNL\Pdk\App\Cart\Model\PdkCart; use MyParcelNL\Pdk\App\DeliveryOptions\Contract\DeliveryOptionsServiceInterface; +use MyParcelNL\Pdk\App\Order\Contract\PdkOrderNoteRepositoryInterface; use MyParcelNL\Pdk\Carrier\Collection\CarrierCollection; use MyParcelNL\Pdk\Carrier\Model\Carrier; use MyParcelNL\Pdk\Context\Model\CheckoutContext; use MyParcelNL\Pdk\Facade\Pdk; use MyParcelNL\Pdk\Settings\Model\CarrierSettings; +use MyParcelNL\Pdk\Settings\Model\LabelSettings; use MyParcelNL\Pdk\Shipment\Model\ShipmentOptions; +use MyParcelNL\Pdk\Tests\Bootstrap\MockStrictPdkOrderNoteRepository; use MyParcelNL\Pdk\Tests\Uses\UsesAccountMock; use MyParcelNL\Pdk\Tests\Uses\UsesMockPdkInstance; use MyParcelNL\Pdk\Types\Service\TriStateService; +use function DI\autowire; use function MyParcelNL\Pdk\Tests\factory; use function MyParcelNL\Pdk\Tests\usesShared; uses()->group('checkout'); -usesShared(new UsesMockPdkInstance(), new UsesAccountMock()); +// The note repository rejects orders without an identifier, like the WooCommerce and +// PrestaShop ones do, so a lookup for the cart's in-memory order fails these tests instead of +// passing silently. +usesShared( + new UsesMockPdkInstance([ + PdkOrderNoteRepositoryInterface::class => autowire(MockStrictPdkOrderNoteRepository::class), + ]), + new UsesAccountMock() +); /** * A deliverable single-line cart with an NL shipping address — the minimum a capabilities @@ -138,3 +150,20 @@ function storeShopWithCarrier(string $carrierName, callable $settingsCallback = ->and($options->ageCheck)->toBe(TriStateService::DISABLED) ->and($options->signature)->toBe(TriStateService::DISABLED); }); + +it('calculates the options while the label description asks for the customer note', function () { + storeShopWithCarrier('POSTNL'); + + // [CUSTOMER_NOTE] makes the label description calculator read the order's notes, which is + // how this broke the checkout: the cart's order has no identifier to look notes up by. + factory(LabelSettings::class) + ->withDescription('[CUSTOMER_NOTE]') + ->store(); + + /** @var \MyParcelNL\Pdk\App\DeliveryOptions\Contract\DeliveryOptionsServiceInterface $service */ + $service = Pdk::get(DeliveryOptionsServiceInterface::class); + + $result = $service->createCartShipmentOptions(cartWithNlAddress()); + + expect($result->get('postnl'))->toBeInstanceOf(ShipmentOptions::class); +}); diff --git a/tests/Unit/App/Order/Model/PdkOrderTest.php b/tests/Unit/App/Order/Model/PdkOrderTest.php index d1f388709..cc6133416 100644 --- a/tests/Unit/App/Order/Model/PdkOrderTest.php +++ b/tests/Unit/App/Order/Model/PdkOrderTest.php @@ -5,6 +5,7 @@ namespace MyParcelNL\Pdk\App\Order\Model; +use MyParcelNL\Pdk\App\Order\Contract\PdkOrderNoteRepositoryInterface; use MyParcelNL\Pdk\App\Order\Collection\PdkOrderCollection; use MyParcelNL\Pdk\App\Order\Collection\PdkOrderCollectionFactory; use MyParcelNL\Pdk\App\Order\Collection\PdkOrderLineCollection; @@ -13,8 +14,11 @@ use MyParcelNL\Pdk\Fulfilment\Model\Order; use MyParcelNL\Pdk\Shipment\Collection\ShipmentCollection; use MyParcelNL\Pdk\Shipment\Model\Shipment; +use MyParcelNL\Pdk\Storage\MemoryCacheStorage; +use MyParcelNL\Pdk\Tests\Bootstrap\MockStrictPdkOrderNoteRepository; use MyParcelNL\Pdk\Tests\Uses\UsesMockPdkInstance; use function MyParcelNL\Pdk\Tests\factory; +use function MyParcelNL\Pdk\Tests\mockPdkProperty; use function MyParcelNL\Pdk\Tests\usesShared; use function Spatie\Snapshots\assertMatchesJsonSnapshot; use MyParcelNL\Pdk\Tests\Uses\UsesAccountMock; @@ -219,3 +223,19 @@ 'result' => false, ], ]); + +it('has no notes when the order only exists in memory', function () { + // Notes are looked up in the shop by order identifier. This repository rejects a missing + // identifier like the WooCommerce and PrestaShop ones do, so the test fails if the model + // asks for them anyway — which used to break the checkout through the cart's order. + $reset = mockPdkProperty( + PdkOrderNoteRepositoryInterface::class, + new MockStrictPdkOrderNoteRepository(new MemoryCacheStorage()) + ); + + $order = new PdkOrder(['lines' => []]); + + expect($order->notes->isEmpty())->toBeTrue(); + + $reset(); +}); From 428792bfa9d3755fde13e51942581a29e60a486b Mon Sep 17 00:00:00 2001 From: Freek van Rijt Date: Thu, 30 Jul 2026 09:35:12 +0200 Subject: [PATCH 4/4] fix(checkout): do not crash when failing to calculate shipment options from the cart the application should degrade gracefully if the shipment options from a pending cart cannot be calculated for whatever reason --- .../Service/DeliveryOptionsService.php | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index bebd8cf57..243e06e4c 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -21,6 +21,7 @@ use MyParcelNL\Pdk\Carrier\Model\Carrier; use MyParcelNL\Pdk\Carrier\Service\CapabilitiesValidationService; use MyParcelNL\Pdk\Facade\FrontendData; +use MyParcelNL\Pdk\Facade\Logger; use MyParcelNL\Pdk\Facade\Pdk; use MyParcelNL\Pdk\Facade\Settings; use MyParcelNL\Pdk\Settings\Model\CarrierSettings; @@ -32,6 +33,7 @@ use MyParcelNL\Sdk\Client\Generated\CoreApi\Model\RefShipmentPackageTypeV2; use MyParcelNL\Sdk\Client\Generated\CoreApi\Model\RefTypesDeliveryTypeV2; use MyParcelNL\Sdk\Support\Str; +use Throwable; class DeliveryOptionsService implements DeliveryOptionsServiceInterface { @@ -175,10 +177,22 @@ public function createCartShipmentOptions(PdkCart $cart): ShipmentOptionsCollect continue; } - // Use the legacy identifier, matching the carrierSettings keys in the config. - $identifier = FrontendData::getLegacyCarrierIdentifier($carrier->carrier); - - $cartShipmentOptions->put($identifier, $this->calculateCartShipmentOptions($carrier, $cart, $packageType)); + /* + * The config here is only passed to the JS Context for the delivery options. + * A failure should not be fatal: the delivery options degrades gracefully and will continue functioning without applying shipment option restrictions from the merchant. + */ + try { + // Use the legacy identifier, matching the carrierSettings keys in the config. + $identifier = FrontendData::getLegacyCarrierIdentifier($carrier->carrier); + + $cartShipmentOptions->put($identifier, $this->calculateCartShipmentOptions($carrier, $cart, $packageType)); + } catch (Throwable $e) { + Logger::error('An error occured when trying to calculate a pending carts shipment options for the delivery options', [ + 'carrier' => $carrier->carrier, + 'error' => $e->getMessage(), + 'trace' => $e->getTraceAsString(), + ]); + } } return $cartShipmentOptions;