From 57474ba095e799fee347a7baf996914c4d23efd1 Mon Sep 17 00:00:00 2001 From: Julien Gissinger Date: Wed, 4 Sep 2024 14:51:46 +0200 Subject: [PATCH] chore(carrier): update and add behats tests --- .../CommandHandler/AddCarrierHandler.php | 13 +++ .../CommandHandler/EditCarrierHandler.php | 8 ++ .../Exception/CarrierConstraintException.php | 5 + .../Domain/Carrier/CarrierFeatureContext.php | 38 +++++++- .../Carrier/CarrierRangesFeatureContext.php | 14 --- .../Carrier/carrier_management.feature | 95 +++++++++++++++++++ 6 files changed, 158 insertions(+), 15 deletions(-) diff --git a/src/Adapter/Carrier/CommandHandler/AddCarrierHandler.php b/src/Adapter/Carrier/CommandHandler/AddCarrierHandler.php index 89b5b51502c92..ac17321f41997 100644 --- a/src/Adapter/Carrier/CommandHandler/AddCarrierHandler.php +++ b/src/Adapter/Carrier/CommandHandler/AddCarrierHandler.php @@ -36,6 +36,7 @@ use PrestaShop\PrestaShop\Core\CommandBus\Attributes\AsCommandHandler; use PrestaShop\PrestaShop\Core\Domain\Carrier\Command\AddCarrierCommand; use PrestaShop\PrestaShop\Core\Domain\Carrier\CommandHandler\AddCarrierHandlerInterface; +use PrestaShop\PrestaShop\Core\Domain\Carrier\Exception\CarrierConstraintException; use PrestaShop\PrestaShop\Core\Domain\Carrier\ValueObject\CarrierId; /** @@ -57,6 +58,8 @@ public function __construct( */ public function handle(AddCarrierCommand $command): CarrierId { + $this->assertZonesExist($command->getZones()); + $carrier = new Carrier(); // General information $carrier->name = $command->getName(); @@ -109,4 +112,14 @@ public function handle(AddCarrierCommand $command): CarrierId return $carrierId; } + + private function assertZonesExist(array $zoneIds): void + { + if (null === $zoneIds || count($zoneIds) === 0) { + throw new CarrierConstraintException( + 'Carrier need to have at least one zone', + CarrierConstraintException::INVALID_ZONE_MISSING + ); + } + } } diff --git a/src/Adapter/Carrier/CommandHandler/EditCarrierHandler.php b/src/Adapter/Carrier/CommandHandler/EditCarrierHandler.php index 4637a97ed0c5e..071fececd62a6 100644 --- a/src/Adapter/Carrier/CommandHandler/EditCarrierHandler.php +++ b/src/Adapter/Carrier/CommandHandler/EditCarrierHandler.php @@ -37,6 +37,7 @@ use PrestaShop\PrestaShop\Core\Domain\Carrier\CommandHandler\EditCarrierHandlerInterface; use PrestaShop\PrestaShop\Core\Domain\Carrier\Exception\CannotUpdateCarrierException; use PrestaShop\PrestaShop\Core\Domain\Carrier\ValueObject\CarrierId; +use PrestaShop\PrestaShop\Core\Domain\Carrier\Exception\CarrierConstraintException; use PrestaShop\PrestaShop\Core\Domain\Shop\ValueObject\ShopId; /** @@ -159,6 +160,13 @@ public function handle(EditCarrierCommand $command): CarrierId } } + if (null === $command->getZones() || count($command->getZones()) === 0) { + throw new CarrierConstraintException( + 'Carrier need to have at least one zone', + CarrierConstraintException::INVALID_ZONE_MISSING + ); + } + $this->carrierRepository->updateAssociatedZones($newCarrierId, $command->getZones()); return $newCarrierId; diff --git a/src/Core/Domain/Carrier/Exception/CarrierConstraintException.php b/src/Core/Domain/Carrier/Exception/CarrierConstraintException.php index 1da5623aab7d0..c028206ec5971 100644 --- a/src/Core/Domain/Carrier/Exception/CarrierConstraintException.php +++ b/src/Core/Domain/Carrier/Exception/CarrierConstraintException.php @@ -137,4 +137,9 @@ class CarrierConstraintException extends CarrierException * Thrown when shop constraint isn't valid */ public const INVALID_SHOP_CONSTRAINT = 200; + + /** + * Thrown when carrier is save without at least one zone + */ + public const INVALID_ZONE_MISSING = 210; } diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Carrier/CarrierFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Carrier/CarrierFeatureContext.php index 6a72fdd8cff4e..ceb749ce7aa58 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Carrier/CarrierFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Carrier/CarrierFeatureContext.php @@ -63,6 +63,7 @@ public static function restoreCarrierTablesAfterSuite(): void public function createCarrier(string $reference, TableNode $node): void { $properties = $this->localizeByRows($node); + try { if (isset($properties['logoPathName']) && 'null' !== $properties['logoPathName']) { $tmpLogo = DummyFileUploader::upload($properties['logoPathName']); @@ -81,6 +82,16 @@ public function createCarrier(string $reference, TableNode $node): void $delay = [$this->getDefaultLangId() => 'Shipping delay']; } + if (!empty($properties['zones'])) { + $getZone = $this->referencesToIds($properties['zones']); + $zones = []; + foreach($getZone as $zoneId) { + array_push($zones, $zoneId->id); + } + } else { + $zones = []; + } + if (!empty($properties['group_access'])) { $groupIds = $this->referencesToIds($properties['group_access']); } else { @@ -103,6 +114,7 @@ public function createCarrier(string $reference, TableNode $node): void $properties['shippingMethod'] ?? 'price', $properties['rangeBehavior'] ?? 'highest_range', $properties['logoPathName'] ?? null, + $zones, $associatedShops, isset($properties['position']) ? (int) $properties['position'] : null, ); @@ -182,7 +194,6 @@ private function editCarrier(string $reference, ?string $newReference, TableNode $carrierId = $this->referenceToId($reference); $command = new EditCarrierCommand($carrierId); - // General information if (isset($properties['name'])) { $command->setName($properties['name']); @@ -217,6 +228,18 @@ private function editCarrier(string $reference, ?string $newReference, TableNode if (isset($properties['group_access'])) { $command->setAssociatedGroupIds($this->referencesToIds($properties['group_access'])); } + + if (isset($properties['zones'])) { + $getZone = $this->referencesToIds($properties['zones']); + $zones = []; + foreach($getZone as $zoneId) { + array_push($zones, $zoneId->id); + } + $command->setZones($zones); + } else { + $command->setZones([]); + } + if (isset($properties['associatedShops'])) { $command->setAssociatedShopIds($this->referencesToIds($properties['associatedShops'])); } @@ -388,6 +411,17 @@ public function carrierEditShouldThrowAnError(string $errorCode) ); } + /** + * @Then carrier create should throw an error with error code :errorCode + */ + public function carrierCreateShouldThrowAnError(string $errorCode) + { + $this->assertLastErrorIs( + CarrierConstraintException::class, + constant(CarrierConstraintException::class . '::' . $errorCode) + ); + } + private function createCarrierUsingCommand( string $name, array $delay, @@ -404,6 +438,7 @@ private function createCarrierUsingCommand( string $shippingMethod, string $rangeBehavior, ?string $logoPathName, + array $zones, array $associatedShops, ?int $position, ): CarrierId { @@ -418,6 +453,7 @@ private function createCarrierUsingCommand( $isFree, $this->convertShippingMethodToInt($shippingMethod), $this->convertOutOfRangeBehaviorToInt($rangeBehavior), + $zones, $associatedShops, $max_width, $max_height, diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Carrier/CarrierRangesFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Carrier/CarrierRangesFeatureContext.php index f4b85f6d4463d..e0318786b3e69 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Carrier/CarrierRangesFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Carrier/CarrierRangesFeatureContext.php @@ -148,20 +148,6 @@ private function getCarrierRanges(string $reference, ShopConstraint $shopConstra $rangesExpected = new CarrierRangesCollection($data); Assert::assertEquals($rangesExpected, $rangesDatabase); - - // Automatically checks that the carrier_zone association is properly set - $query = new DbQuery(); - $query->select('(cz.id_zone)'); - $query->from('carrier_zone', 'cz'); - $query->where('id_carrier = \'' . pSQL((string) $carrierId) . '\''); - $zonesFromDB = Db::getInstance(_PS_USE_SQL_SLAVE_)->executeS($query->build()); - - $zoneIds = array_unique($zoneIds); - sort($zoneIds); - $zonesFromDB = array_map(fn ($row) => $row['id_zone'], $zonesFromDB); - sort($zonesFromDB); - - Assert::assertEquals($zoneIds, $zonesFromDB); } catch (CarrierException $e) { $this->setLastException($e); } diff --git a/tests/Integration/Behaviour/Features/Scenario/Carrier/carrier_management.feature b/tests/Integration/Behaviour/Features/Scenario/Carrier/carrier_management.feature index d546ba175a35a..15e951a3569cb 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Carrier/carrier_management.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Carrier/carrier_management.feature @@ -12,6 +12,12 @@ Feature: Carrier management Background: Given group "visitor" named "Visitor" exists Given group "guest" named "Guest" exists + Given I add new zone "zone1" with following properties: + | name | zone1 | + | enabled | true | + Given I add new zone "zone2" with following properties: + | name | zone2 | + | enabled | true | And language "en" with locale "en-US" exists And language "fr" with locale "fr-FR" exists And language with iso code "en" is the default one @@ -32,6 +38,7 @@ Feature: Carrier management | shippingHandling | false | | isFree | true | | shippingMethod | weight | + | zones | zone1 | | rangeBehavior | disabled | Then carrier "carrier1" should have the following properties: | name | Carrier 1 | @@ -50,6 +57,7 @@ Feature: Carrier management | isFree | true | | shippingMethod | weight | | rangeBehavior | disabled | + | zones | zone1 | | ordersCount | 0 | Then carrier "carrier1" shouldn't have a logo @@ -82,6 +90,7 @@ Feature: Carrier management | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | | shippingHandling | false | + | zones | zone2 | | isFree | true | | shippingMethod | weight | | taxRuleGroup | US-AL Rate (4%) | @@ -110,6 +119,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -129,6 +139,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -164,6 +175,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -201,6 +213,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -239,6 +252,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -293,6 +307,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -330,6 +345,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -368,6 +384,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -408,6 +425,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -445,6 +463,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | false | | shippingMethod | weight | @@ -468,6 +487,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | false | | shippingMethod | weight | @@ -491,6 +511,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | false | | shippingMethod | weight | @@ -514,6 +535,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | true | | isFree | false | | shippingMethod | weight | @@ -561,6 +583,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | false | | shippingMethod | weight | @@ -584,6 +607,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | true | | isFree | false | | shippingMethod | weight | @@ -605,6 +629,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -628,6 +653,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | true | | isFree | false | | shippingMethod | weight | @@ -651,6 +677,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | true | | isFree | false | | shippingMethod | weight | @@ -673,6 +700,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -693,6 +721,7 @@ Feature: Carrier management | group_access | visitor, guest | | delay[en-US] | Shipping delay | | delay[fr-FR] | Délai de livraison | + | zones | zone2 | | shippingHandling | false | | isFree | true | | shippingMethod | weight | @@ -702,3 +731,69 @@ Feature: Carrier management When I edit carrier "carrier1" with specified properties: | logoPathName | | Then carrier "carrier1" shouldn't have a logo + + Scenario: Partially editing carrier with zones + When I create carrier "carrier1" with specified properties: + | name | Carrier 1 | + | grade | 1 | + | trackingUrl | http://example.com/track.php?num=@ | + | active | true | + | max_width | 1454 | + | max_height | 1234 | + | max_depth | 1111 | + | max_weight | 3864 | + | group_access | visitor, guest | + | delay[en-US] | Shipping delay | + | delay[fr-FR] | Délai de livraison | + | shippingHandling | false | + | zones | zone1, zone2 | + | isFree | false | + | shippingMethod | weight | + | rangeBehavior | disabled | + When I edit carrier "carrier1" with specified properties: + | zones | zone1 | + Then carrier "carrier1" should have the following properties: + | name | Carrier 1 | + | zones | zone1 | + + Scenario: Add a new carrier without any zone + When I create carrier "carrier1" with specified properties: + | name | Carrier 1 | + | grade | 1 | + | trackingUrl | http://example.com/track.php?num=@ | + | active | true | + | max_width | 1454 | + | max_height | 1234 | + | max_depth | 1111 | + | max_weight | 3864 | + | group_access | visitor, guest | + | delay[en-US] | Shipping delay | + | delay[fr-FR] | Délai de livraison | + | shippingHandling | false | + | isFree | false | + | shippingMethod | weight | + | rangeBehavior | disabled | + Then carrier create should throw an error with error code "INVALID_ZONE_MISSING" + + Scenario: Edit a new carrier and delete all zone + When I create carrier "carrier1" with specified properties: + | name | Carrier 1 | + | grade | 1 | + | trackingUrl | http://example.com/track.php?num=@ | + | active | true | + | max_width | 1454 | + | max_height | 1234 | + | max_depth | 1111 | + | max_weight | 3864 | + | group_access | visitor, guest | + | delay[en-US] | Shipping delay | + | delay[fr-FR] | Délai de livraison | + | zones | zone1, zone2 | + | shippingHandling | false | + | isFree | false | + | shippingMethod | weight | + | rangeBehavior | disabled | + When I edit carrier "carrier1" with specified properties: + | zones | | + Then carrier edit should throw an error with error code "INVALID_ZONE_MISSING" +