diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index fb6480d28..a57db58de 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -15,6 +15,7 @@ use MyParcelNL\Pdk\Base\Support\Collection; use MyParcelNL\Pdk\Base\Support\SettingKey; use MyParcelNL\Pdk\Base\Support\Utils; +use MyParcelNL\Pdk\Carrier\Collection\CarrierCollection; use MyParcelNL\Pdk\Carrier\Contract\CarrierRepositoryInterface; use MyParcelNL\Pdk\Carrier\Model\Carrier; use MyParcelNL\Pdk\Carrier\Service\CapabilitiesValidationService; @@ -229,8 +230,18 @@ private function getBaseSettings(CarrierSettings $carrierSettings, PdkCart $cart */ private function getValidCarrierOptions(PdkCart $cart): array { + $carrierSettings = Settings::get(CarrierSettings::ID); + + $carrierSettings = array_filter( + is_array($carrierSettings) ? $carrierSettings : [], + static fn($settings): bool => is_array($settings) + ); + + if (empty($carrierSettings)) { + return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()]; + } + $allCarriers = $this->carrierRepository->all(); - $carrierSettings = Settings::get(CarrierSettings::ID); $shippingAddress = $cart->shippingMethod->shippingAddress; $cc = $shippingAddress->cc ?? null; $isBusiness = $shippingAddress->isBusiness; diff --git a/src/Carrier/Service/CapabilitiesValidationService.php b/src/Carrier/Service/CapabilitiesValidationService.php index 4c8299018..2dc6d942d 100644 --- a/src/Carrier/Service/CapabilitiesValidationService.php +++ b/src/Carrier/Service/CapabilitiesValidationService.php @@ -133,11 +133,16 @@ private function getEnabledCarrierNames(): array { $carrierSettings = Settings::get(CarrierSettings::ID) ?? []; + if (! is_array($carrierSettings)) { + return []; + } + return array_keys( array_filter( $carrierSettings, static function ($settings): bool { - return ! empty($settings[CarrierSettings::DELIVERY_OPTIONS_ENABLED]); + return is_array($settings) + && ! empty($settings[CarrierSettings::DELIVERY_OPTIONS_ENABLED]); } ) ); diff --git a/src/Settings/Repository/AbstractPdkSettingsRepository.php b/src/Settings/Repository/AbstractPdkSettingsRepository.php index 121ba1da3..7e61f9db1 100644 --- a/src/Settings/Repository/AbstractPdkSettingsRepository.php +++ b/src/Settings/Repository/AbstractPdkSettingsRepository.php @@ -137,10 +137,18 @@ protected function updateSettingsFromCollection( ): Settings { $category = $this->get($this->createSettingsKey($settingsId)) ?? []; - foreach ($category as $key => $item) { - $values = ['id' => $key] + $this->get($this->createSettingsKey("$settingsId.$key")); + if (! is_array($category)) { + $category = []; + } + + foreach (array_keys($category) as $key) { + $values = $this->get($this->createSettingsKey("$settingsId.$key")); + + if (! is_array($values)) { + continue; + } - $collection->offsetSet($key, $values); + $collection->offsetSet($key, ['id' => $key] + $values); } $settings->setAttribute($settingsId, $collection); diff --git a/tests/Unit/App/Context/Model/DeliveryOptionsConfigTest.php b/tests/Unit/App/Context/Model/DeliveryOptionsConfigTest.php index 24712d07e..49d46340c 100644 --- a/tests/Unit/App/Context/Model/DeliveryOptionsConfigTest.php +++ b/tests/Unit/App/Context/Model/DeliveryOptionsConfigTest.php @@ -15,6 +15,7 @@ use MyParcelNL\Pdk\Facade\Pdk; use MyParcelNL\Pdk\Facade\Settings; use MyParcelNL\Pdk\Proposition\Proposition; +use MyParcelNL\Pdk\Settings\Model\CarrierSettings; use MyParcelNL\Pdk\Settings\Model\CheckoutSettings; use MyParcelNL\Pdk\Tests\Bootstrap\MockPdkProductRepository; use MyParcelNL\Pdk\Tests\Bootstrap\TestBootstrapper; @@ -135,6 +136,10 @@ ->withAllowPickupLocationsViewSelection(true) ->store(); + factory(CarrierSettings::class, RefCapabilitiesSharedCarrierV2::POSTNL) + ->withDeliveryOptionsEnabled(true) + ->store(); + /** @var \MyParcelNL\Pdk\Tests\Bootstrap\MockPdkProductRepository $productRepository */ $productRepository = Pdk::get(PdkProductRepositoryInterface::class); diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCapabilitiesTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCapabilitiesTest.php index 36c791420..6c4df49a8 100644 --- a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCapabilitiesTest.php +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCapabilitiesTest.php @@ -13,6 +13,7 @@ use MyParcelNL\Pdk\Carrier\Model\Carrier; use MyParcelNL\Pdk\Facade\FrontendData; use MyParcelNL\Pdk\Facade\Pdk; +use MyParcelNL\Pdk\Settings\Contract\PdkSettingsRepositoryInterface; use MyParcelNL\Pdk\Settings\Model\CarrierSettings; use MyParcelNL\Pdk\Settings\Model\Settings; use MyParcelNL\Pdk\Shipment\Model\DeliveryOptions; @@ -307,6 +308,42 @@ function enqueueCapabilitiesPerType(array $responsesPerType): void ->and($result['carrierSettings'])->not->toHaveKey($disabledId); }); +it('keeps valid carriers when another carrier setting is malformed', function () { + $carrierName = RefCapabilitiesSharedCarrierV2::getAllowableEnumValues()[0]; + + storeCarrierSettings([$carrierName => true]); + + /** @var \MyParcelNL\Pdk\Settings\Contract\PdkSettingsRepositoryInterface $settingsRepository */ + $settingsRepository = Pdk::get(PdkSettingsRepositoryInterface::class); + $settingsKey = Pdk::get('createSettingsKey')(CarrierSettings::ID); + $carrierSettings = $settingsRepository->get($settingsKey); + + $settingsRepository->store($settingsKey, array_merge($carrierSettings, ['invalid' => 'invalid'])); + + factory(Shop::class) + ->withCarriers( + factory(CarrierCollection::class) + ->push(factory(Carrier::class) + ->withCarrier($carrierName) + ->withCapabilityPackageTypes(['PACKAGE'])) + ) + ->store(); + + resetStorageCache(); + + enqueueCapabilitiesPerType([ + 'PACKAGE' => [capabilityResult($carrierName, 100, ['PACKAGE'])], + ]); + + /** @var DeliveryOptionsServiceInterface $service */ + $service = Pdk::get(DeliveryOptionsServiceInterface::class); + $result = $service->createAllCarrierSettings(makeCart('NL')); + + $carrierId = FrontendData::getLegacyCarrierIdentifier($carrierName); + + expect($result['carrierSettings'])->toHaveKey($carrierId); +}); + it('passes contract ID from capabilities to carrier settings output', function () { storeCarrierSettings([RefCapabilitiesSharedCarrierV2::POSTNL => true]); @@ -476,4 +513,3 @@ function enqueueCapabilitiesPerType(array $responsesPerType): void expect($result['packageType'])->toBe(DeliveryOptions::PACKAGE_TYPE_MAILBOX_NAME); }); - diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php new file mode 100644 index 000000000..21df8e139 --- /dev/null +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php @@ -0,0 +1,66 @@ +group('checkout'); + +usesShared(new UsesMockPdkInstance(), new UsesAccountMock()); + +it('does not expose carriers when carrier settings are missing or invalid', function ($carrierSettings) { + $settingsManager = Mockery::mock(SettingsManagerInterface::class); + $settingsManager + ->shouldReceive('get') + ->andReturnUsing(static function ( + string $key, + ?string $namespace = null, + $default = null + ) use ($carrierSettings) { + return CarrierSettings::ID === $key && null === $namespace + ? $carrierSettings + : $default; + }); + Pdk::set(SettingsManagerInterface::class, $settingsManager); + + /** @var DeliveryOptionsServiceInterface $service */ + $service = Pdk::get(DeliveryOptionsServiceInterface::class); + + $result = $service->createAllCarrierSettings(new PdkCart([ + 'shippingMethod' => [ + 'shippingAddress' => ['cc' => 'NL'], + ], + 'lines' => [ + [ + 'quantity' => 1, + 'product' => [ + 'weight' => 1000, + 'isDeliverable' => true, + ], + ], + ], + ])); + + expect($result['packageType'])->toBe(DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME) + ->and($result['carrierSettings'])->toBe([]); +})->with([ + 'missing' => [null], + 'empty array' => [[]], + 'malformed carrier entry' => [['carrier' => 'invalid']], + 'boolean value' => [false], + 'string value' => ['invalid'], +]); diff --git a/tests/Unit/Settings/Repository/AbstractSettingsRepositoryTest.php b/tests/Unit/Settings/Repository/AbstractSettingsRepositoryTest.php index eb66121ad..efb1d5b13 100644 --- a/tests/Unit/Settings/Repository/AbstractSettingsRepositoryTest.php +++ b/tests/Unit/Settings/Repository/AbstractSettingsRepositoryTest.php @@ -49,6 +49,28 @@ assertMatchesJsonSnapshot(json_encode($settings->toArrayWithoutNull())); }); +it('skips malformed entries when retrieving collection settings', function () { + /** @var \MyParcelNL\Pdk\Settings\Contract\PdkSettingsRepositoryInterface $repository */ + $repository = Pdk::get(PdkSettingsRepositoryInterface::class); + $createSettingsKey = Pdk::get('createSettingsKey'); + + $currentCarrierSettings = $repository->get($createSettingsKey(CarrierSettings::ID)); + + try { + $repository->store($createSettingsKey(CarrierSettings::ID), array_merge($currentCarrierSettings, [ + 'valid' => [CarrierSettings::DELIVERY_OPTIONS_ENABLED => true], + 'invalid' => 'invalid', + ])); + + $carrierSettings = $repository->all()->carrier; + + expect($carrierSettings->has('valid'))->toBeTrue() + ->and($carrierSettings->has('invalid'))->toBeFalse(); + } finally { + $repository->store($createSettingsKey(CarrierSettings::ID), $currentCarrierSettings); + } +}); + it('retrieves a single setting from a category', function (string $key, $expected) { /** @var \MyParcelNL\Pdk\Settings\Contract\PdkSettingsRepositoryInterface $repository */ $repository = Pdk::get(PdkSettingsRepositoryInterface::class);