From ce3ca849bae7beb15b3fadc7d2971c65f92a5e9f Mon Sep 17 00:00:00 2001 From: Florian de Vries Date: Tue, 17 Sep 2024 17:07:31 +0200 Subject: [PATCH 1/7] fix(checkout): fix problem that prevents pickup price from being taken into consideration --- src/Hooks/HasPsShippingCostHooks.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Hooks/HasPsShippingCostHooks.php b/src/Hooks/HasPsShippingCostHooks.php index a80aa21b..05169f13 100644 --- a/src/Hooks/HasPsShippingCostHooks.php +++ b/src/Hooks/HasPsShippingCostHooks.php @@ -10,6 +10,7 @@ use MyParcelNL\Pdk\Facade\Pdk; use MyParcelNL\Pdk\Shipment\Model\DeliveryOptions; use MyParcelNL\PrestaShop\Contract\PsCarrierServiceInterface; +use MyParcelNL\PrestaShop\Repository\PsCartDeliveryOptionsRepository; use Tools; trait HasPsShippingCostHooks @@ -101,6 +102,15 @@ private function getDeliveryOptions(Carrier $carrier): DeliveryOptions return new DeliveryOptions(json_decode($deliveryOptions, true)); } + //Delivery options are not in hidden input, fetch them from the database. + /** @var PsCartDeliveryOptionsRepository $cartDeliveryOptionsRepository */ + $cartDeliveryOptionsRepository = Pdk::get(PsCartDeliveryOptionsRepository::class); + $deliveryOptions = $cartDeliveryOptionsRepository->findOneBy(['cartId' => $this->context->cart->id]); + + if ($deliveryOptions && property_exists($deliveryOptions, 'data')) { + return new DeliveryOptions($deliveryOptions->getData()); + } + return new DeliveryOptions(['carrier' => $carrier]); } } From 851bad926c5ffeb50d0978b5f6202718eadc5fef Mon Sep 17 00:00:00 2001 From: Florian de Vries Date: Wed, 18 Sep 2024 12:00:11 +0200 Subject: [PATCH 2/7] fix(checkout): use only deliveryoptions in db if it belongs to the current carrier --- src/Hooks/HasPsShippingCostHooks.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Hooks/HasPsShippingCostHooks.php b/src/Hooks/HasPsShippingCostHooks.php index 05169f13..d321663f 100644 --- a/src/Hooks/HasPsShippingCostHooks.php +++ b/src/Hooks/HasPsShippingCostHooks.php @@ -88,7 +88,8 @@ private function calculateShippingCost(DeliveryOptions $deliveryOptions, float $ } /** - * Get the actual delivery options from the checkout hidden input, or get the default delivery options for the current carrier, without any options. + * Get the actual delivery options from the checkout hidden input, or from the database using the cart id, + * or get the default delivery options for the current carrier, without any options. * * @param \MyParcelNL\Pdk\Carrier\Model\Carrier $carrier * @@ -107,7 +108,11 @@ private function getDeliveryOptions(Carrier $carrier): DeliveryOptions $cartDeliveryOptionsRepository = Pdk::get(PsCartDeliveryOptionsRepository::class); $deliveryOptions = $cartDeliveryOptionsRepository->findOneBy(['cartId' => $this->context->cart->id]); - if ($deliveryOptions && property_exists($deliveryOptions, 'data')) { + $useDbDeliveryOptions = $deliveryOptions && property_exists($deliveryOptions, 'data'); + $useDbDeliveryOptions = $useDbDeliveryOptions && $deliveryOptions->getData()['carrier']['externalIdentifier'] === $carrier->externalIdentifier; + + if ($useDbDeliveryOptions) { + //todo coverage verhogen return new DeliveryOptions($deliveryOptions->getData()); } From 14e5cb21a7f11b3b75027756561577d050cdcf4f Mon Sep 17 00:00:00 2001 From: Florian de Vries Date: Wed, 18 Sep 2024 16:42:42 +0200 Subject: [PATCH 3/7] test: create tests for delivery options saved in database --- .../Unit/Hooks/HasPsShippingCostHooksTest.php | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/tests/Unit/Hooks/HasPsShippingCostHooksTest.php b/tests/Unit/Hooks/HasPsShippingCostHooksTest.php index aa702529..b3d5cd7b 100644 --- a/tests/Unit/Hooks/HasPsShippingCostHooksTest.php +++ b/tests/Unit/Hooks/HasPsShippingCostHooksTest.php @@ -18,6 +18,7 @@ use MyParcelNL\Pdk\Shipment\Model\ShipmentOptions; use MyParcelNL\Pdk\Tests\Factory\Collection\FactoryCollection; use MyParcelNL\PrestaShop\Entity\MyparcelnlCarrierMapping; +use MyParcelNL\PrestaShop\Repository\PsCartDeliveryOptionsRepository; use MyParcelNL\PrestaShop\Tests\Mock\MockPsTools; use MyParcelNL\PrestaShop\Tests\Uses\UsesMockPsPdkInstance; use function MyParcelNL\Pdk\Tests\factory; @@ -44,6 +45,11 @@ public function setIdCarrier(int $idCarrier): void { $this->id_carrier = $idCarrier; } + + public function setCart(Cart $cart): void + { + $this->context->cart = $cart; + } } usesShared(new UsesMockPsPdkInstance()); @@ -137,3 +143,80 @@ function () { }); +it('calculates shipping costs using database', function (CartFactory $cartFactory, array $deliveryOptions = [], float $addedCost = 0) { + $cart = $cartFactory->make(); + + /** @var PsCartDeliveryOptionsRepository $cartDeliveryOptionsRepository */ + $cartDeliveryOptionsRepository = Pdk::get(PsCartDeliveryOptionsRepository::class); + + $deliveryOptions = new DeliveryOptions($deliveryOptions); + + $cartDeliveryOptionsRepository->updateOrCreate( + [ + 'cartId' => $cart->id, + ], + [ + 'data' => json_encode($deliveryOptions->toStorableArray()), + ] + ); + + $instance = new ClassWithTrait(); + $instance->setIdCarrier(93); + $instance->setCart($cart); + + $baseCost = 10; + $cost = $instance->getOrderShippingCost($cart, $baseCost); + + expect(number_format($cost, 2))->toEqual(number_format($baseCost + $addedCost, 2)); +})->with([ + 'standard delivery with delivery options in values' => [ + function () { + $psCarrier = psFactory(PsCarrier::class)->withId(93); + + (new FactoryCollection([ + $psCarrier, + psFactory(MyparcelnlCarrierMapping::class) + ->withCarrierId(93) + ->withMyparcelCarrier(Carrier::CARRIER_POSTNL_NAME), + factory(Settings::class)->withCarrierPostNl([ + CarrierSettings::PRICE_DELIVERY_TYPE_STANDARD => 2.95, + ]), + ]))->store(); + + return psFactory(Cart::class)->withCarrier($psCarrier); + }, + 'values' => [ + DeliveryOptions::CARRIER => Carrier::CARRIER_POSTNL_NAME, + DeliveryOptions::DELIVERY_TYPE => DeliveryOptions::DELIVERY_TYPE_STANDARD_NAME, + ], + 'cost' => 2.95, + ], + + 'carrier with linked myparcel carrier and delivery options in values' => [ + function () { + $psCarrier = psFactory(PsCarrier::class)->withId(93); + + (new FactoryCollection([ + $psCarrier, + psFactory(MyparcelnlCarrierMapping::class) + ->withCarrierId(93) + ->withMyparcelCarrier(Carrier::CARRIER_POSTNL_NAME), + factory(Settings::class) + ->withCarrierPostNl([ + CarrierSettings::PRICE_SIGNATURE => 0.45, + CarrierSettings::PRICE_DELIVERY_TYPE_STANDARD => 4.95, + ]), + ]))->store(); + + return psFactory(Cart::class)->withCarrier($psCarrier); + }, + 'values' => [ + DeliveryOptions::CARRIER => Carrier::CARRIER_POSTNL_NAME, + DeliveryOptions::DELIVERY_TYPE => DeliveryOptions::DELIVERY_TYPE_STANDARD_NAME, + DeliveryOptions::SHIPMENT_OPTIONS => [ + ShipmentOptions::SIGNATURE => true, + ], + ], + 'cost' => 5.4, + ], +]); \ No newline at end of file From 4eed12a5a327676dad1b55c8760c3509172d8da8 Mon Sep 17 00:00:00 2001 From: Florian de Vries Date: Thu, 19 Sep 2024 08:41:30 +0200 Subject: [PATCH 4/7] style: remove todo --- src/Hooks/HasPsShippingCostHooks.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Hooks/HasPsShippingCostHooks.php b/src/Hooks/HasPsShippingCostHooks.php index d321663f..69f3a0df 100644 --- a/src/Hooks/HasPsShippingCostHooks.php +++ b/src/Hooks/HasPsShippingCostHooks.php @@ -112,7 +112,6 @@ private function getDeliveryOptions(Carrier $carrier): DeliveryOptions $useDbDeliveryOptions = $useDbDeliveryOptions && $deliveryOptions->getData()['carrier']['externalIdentifier'] === $carrier->externalIdentifier; if ($useDbDeliveryOptions) { - //todo coverage verhogen return new DeliveryOptions($deliveryOptions->getData()); } From 71d87205d2db789ded4d45ad4bb3b3ed2b3a6df3 Mon Sep 17 00:00:00 2001 From: Florian de Vries Date: Thu, 19 Sep 2024 15:09:38 +0200 Subject: [PATCH 5/7] feat: clear MockPsTools between tests --- tests/Mock/MockPsTools.php | 5 +++++ tests/Pest.php | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/Mock/MockPsTools.php b/tests/Mock/MockPsTools.php index d5364dc6..9ca35be8 100644 --- a/tests/Mock/MockPsTools.php +++ b/tests/Mock/MockPsTools.php @@ -30,6 +30,11 @@ public static function getValue(string $key) return self::$values[$key] ?? null; } + public static function reset(): void + { + self::$values = []; + } + /** * @param array $values * diff --git a/tests/Pest.php b/tests/Pest.php index 7d90b93d..f1584472 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -8,6 +8,7 @@ * @see https://pestphp.com/docs/underlying-test-case#testspestphp */ +use MyParcelNL\PrestaShop\Tests\Mock\MockPsTools; use MyParcelNL\PrestaShop\Tests\TestCase; /** @see \MyParcelNL\PrestaShop\bootPdk() */ @@ -29,7 +30,17 @@ const _PS_VERSION_ = '8.0.0'; const _PS_USE_SQL_SLAVE_ = false; -uses(TestCase::class)->in(__DIR__); +uses(TestCase::class) + ->afterEach(function () { + $resetInterfaces = [ + MockPsTools::class, + ]; + + foreach ($resetInterfaces as $resetInterface) { + $resetInterface::reset(); + } + }) + ->in(__DIR__); uses() ->group('migrations') From a0a9a4415c06c0b506a36437d7047b966e2c995a Mon Sep 17 00:00:00 2001 From: Florian de Vries Date: Thu, 19 Sep 2024 15:33:46 +0200 Subject: [PATCH 6/7] style: change formatting and readability --- src/Hooks/HasPsShippingCostHooks.php | 19 +++---- .../Unit/Hooks/HasPsShippingCostHooksTest.php | 52 ++++++++++--------- 2 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/Hooks/HasPsShippingCostHooks.php b/src/Hooks/HasPsShippingCostHooks.php index 69f3a0df..2af491e9 100644 --- a/src/Hooks/HasPsShippingCostHooks.php +++ b/src/Hooks/HasPsShippingCostHooks.php @@ -103,16 +103,17 @@ private function getDeliveryOptions(Carrier $carrier): DeliveryOptions return new DeliveryOptions(json_decode($deliveryOptions, true)); } - //Delivery options are not in hidden input, fetch them from the database. - /** @var PsCartDeliveryOptionsRepository $cartDeliveryOptionsRepository */ + /** @var \MyParcelNL\PrestaShop\Repository\PsCartDeliveryOptionsRepository $cartDeliveryOptionsRepository */ $cartDeliveryOptionsRepository = Pdk::get(PsCartDeliveryOptionsRepository::class); - $deliveryOptions = $cartDeliveryOptionsRepository->findOneBy(['cartId' => $this->context->cart->id]); - - $useDbDeliveryOptions = $deliveryOptions && property_exists($deliveryOptions, 'data'); - $useDbDeliveryOptions = $useDbDeliveryOptions && $deliveryOptions->getData()['carrier']['externalIdentifier'] === $carrier->externalIdentifier; - - if ($useDbDeliveryOptions) { - return new DeliveryOptions($deliveryOptions->getData()); + $dbDeliveryOptions = $cartDeliveryOptionsRepository->findOneBy( + ['cartId' => $this->context->cart->id] + ); + + if ( + $dbDeliveryOptions + && $dbDeliveryOptions->getData()['carrier']['externalIdentifier'] === $carrier->externalIdentifier + ) { + return new DeliveryOptions($dbDeliveryOptions->getData()); } return new DeliveryOptions(['carrier' => $carrier]); diff --git a/tests/Unit/Hooks/HasPsShippingCostHooksTest.php b/tests/Unit/Hooks/HasPsShippingCostHooksTest.php index b3d5cd7b..f850a1be 100644 --- a/tests/Unit/Hooks/HasPsShippingCostHooksTest.php +++ b/tests/Unit/Hooks/HasPsShippingCostHooksTest.php @@ -142,33 +142,35 @@ function () { expect($result)->toBe(123.45); }); +it( + 'calculates shipping costs using database', + function (CartFactory $cartFactory, array $deliveryOptions = [], float $addedCost = 0) { + $cart = $cartFactory->make(); -it('calculates shipping costs using database', function (CartFactory $cartFactory, array $deliveryOptions = [], float $addedCost = 0) { - $cart = $cartFactory->make(); + /** @var PsCartDeliveryOptionsRepository $cartDeliveryOptionsRepository */ + $cartDeliveryOptionsRepository = Pdk::get(PsCartDeliveryOptionsRepository::class); - /** @var PsCartDeliveryOptionsRepository $cartDeliveryOptionsRepository */ - $cartDeliveryOptionsRepository = Pdk::get(PsCartDeliveryOptionsRepository::class); + $deliveryOptions = new DeliveryOptions($deliveryOptions); - $deliveryOptions = new DeliveryOptions($deliveryOptions); + $cartDeliveryOptionsRepository->updateOrCreate( + [ + 'cartId' => $cart->id, + ], + [ + 'data' => json_encode($deliveryOptions->toStorableArray()), + ] + ); - $cartDeliveryOptionsRepository->updateOrCreate( - [ - 'cartId' => $cart->id, - ], - [ - 'data' => json_encode($deliveryOptions->toStorableArray()), - ] - ); + $instance = new ClassWithTrait(); + $instance->setIdCarrier(93); + $instance->setCart($cart); - $instance = new ClassWithTrait(); - $instance->setIdCarrier(93); - $instance->setCart($cart); + $baseCost = 10; + $cost = $instance->getOrderShippingCost($cart, $baseCost); - $baseCost = 10; - $cost = $instance->getOrderShippingCost($cart, $baseCost); - - expect(number_format($cost, 2))->toEqual(number_format($baseCost + $addedCost, 2)); -})->with([ + expect(number_format($cost, 2))->toEqual(number_format($baseCost + $addedCost, 2)); + } +)->with([ 'standard delivery with delivery options in values' => [ function () { $psCarrier = psFactory(PsCarrier::class)->withId(93); @@ -186,10 +188,10 @@ function () { return psFactory(Cart::class)->withCarrier($psCarrier); }, 'values' => [ - DeliveryOptions::CARRIER => Carrier::CARRIER_POSTNL_NAME, - DeliveryOptions::DELIVERY_TYPE => DeliveryOptions::DELIVERY_TYPE_STANDARD_NAME, + DeliveryOptions::CARRIER => Carrier::CARRIER_POSTNL_NAME, + DeliveryOptions::DELIVERY_TYPE => DeliveryOptions::DELIVERY_TYPE_STANDARD_NAME, ], - 'cost' => 2.95, + 'cost' => 2.95, ], 'carrier with linked myparcel carrier and delivery options in values' => [ @@ -219,4 +221,4 @@ function () { ], 'cost' => 5.4, ], -]); \ No newline at end of file +]); From 45c7674701afbee9fda6b1e76badfcf3e12c4be8 Mon Sep 17 00:00:00 2001 From: Florian de Vries Date: Fri, 20 Sep 2024 15:30:36 +0200 Subject: [PATCH 7/7] fix(checkout): use most recently selected shipping price --- .../listeners/updateCheckoutOnUpdate.ts | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/views/js/frontend/checkout/src/deliveryOptions/listeners/updateCheckoutOnUpdate.ts b/views/js/frontend/checkout/src/deliveryOptions/listeners/updateCheckoutOnUpdate.ts index 16101c4c..4b14dc81 100644 --- a/views/js/frontend/checkout/src/deliveryOptions/listeners/updateCheckoutOnUpdate.ts +++ b/views/js/frontend/checkout/src/deliveryOptions/listeners/updateCheckoutOnUpdate.ts @@ -1,14 +1,14 @@ -import {debounce, type DeliveryOptionsStoreState, type StoreCallbackUpdate} from '@myparcel-pdk/checkout'; +import { + debounce, + useDeliveryOptionsStore, + type DeliveryOptionsStoreState, + type StoreCallbackUpdate, +} from '@myparcel-pdk/checkout'; import {objectIsEqual} from '@myparcel/ts-utils'; import {getCurrentShippingMethod} from '../../utils'; const CHECKOUT_UPDATE_DELAY = 200; -/** - * Only do this once to avoid excessive animations. - */ -let done = false; - export const onDeliveryOptionsOutputChange: StoreCallbackUpdate = debounce( (newState, oldState) => { if (objectIsEqual(newState.output, oldState?.output)) { @@ -21,13 +21,12 @@ export const onDeliveryOptionsOutputChange: StoreCallbackUpdate