From 10c589bf18ea9164f13df5efe74f863d37fa4ad0 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 7 Oct 2020 11:50:37 +0300 Subject: [PATCH 01/18] refactor UpdateProductPricesHandler - use ProductRepository for update --- .../UpdateProductPricesHandler.php | 115 ++++++++++-------- .../config/services/adapter/product.yml | 1 + 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php index c0b4ba590a771..be7954195158d 100644 --- a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php +++ b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php @@ -29,8 +29,10 @@ namespace PrestaShop\PrestaShop\Adapter\Product\CommandHandler; use PrestaShop\Decimal\DecimalNumber; +use PrestaShop\Decimal\Exception\DivisionByZeroException; use PrestaShop\PrestaShop\Adapter\Entity\TaxRulesGroup; use PrestaShop\PrestaShop\Adapter\Product\AbstractProductHandler; +use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler\UpdateProductPricesHandlerInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\CannotUpdateProductException; @@ -52,17 +54,20 @@ final class UpdateProductPricesHandler extends AbstractProductHandler implements private $numberExtractor; /** - * @var Product the product being updated + * @var ProductRepository */ - private $product; + private $productRepository; /** * @param NumberExtractor $numberExtractor + * @param ProductRepository $productRepository */ public function __construct( - NumberExtractor $numberExtractor + NumberExtractor $numberExtractor, + ProductRepository $productRepository ) { $this->numberExtractor = $numberExtractor; + $this->productRepository = $productRepository; } /** @@ -70,43 +75,45 @@ public function __construct( */ public function handle(UpdateProductPricesCommand $command): void { - $product = $this->getProduct($command->getProductId()); - $this->product = $product; - $this->fillUpdatableFieldsWithCommandData($product, $command); - $product->setFieldsToUpdate($this->fieldsToUpdate); + $product = $this->productRepository->get($command->getProductId()); + $updatableProperties = $this->fillUpdatableProperties($product, $command); - if (empty($this->fieldsToUpdate)) { - return; - } - - $this->performUpdate($product, CannotUpdateProductException::FAILED_UPDATE_PRICES); + $this->productRepository->partialUpdate($product, $updatableProperties, CannotUpdateProductException::FAILED_UPDATE_PRICES); } /** * @param Product $product * @param UpdateProductPricesCommand $command * + * @return string[] updatable properties + * * @throws ProductConstraintException */ - private function fillUpdatableFieldsWithCommandData(Product $product, UpdateProductPricesCommand $command): void + private function fillUpdatableProperties(Product $product, UpdateProductPricesCommand $command): array { - if (null !== $command->getPrice()) { - $product->price = (float) (string) $command->getPrice(); + $updatableProperties = []; + + $price = $command->getPrice(); + if (null !== $price) { + $product->price = (float) (string) $price; $this->validateField($product, 'price', ProductConstraintException::INVALID_PRICE); - $this->fieldsToUpdate['price'] = true; + $updatableProperties[] = 'price'; + } else { + $price = new DecimalNumber((string) $product->price); } - $this->handleUnitPriceInfo($command); + $this->fillUnitPriceRatio($product, $price, $command->getUnitPrice()); + $updatableProperties[] = 'unit_price_ratio'; if (null !== $command->getUnity()) { $product->unity = $command->getUnity(); - $this->fieldsToUpdate['unity'] = true; + $updatableProperties[] = 'unity'; } if (null !== $command->getEcotax()) { $product->ecotax = (float) (string) $command->getEcotax(); $this->validateField($product, 'ecotax', ProductConstraintException::INVALID_ECOTAX); - $this->fieldsToUpdate['ecotax'] = true; + $updatableProperties[] = 'ecotax'; } $taxRulesGroupId = $command->getTaxRulesGroupId(); @@ -115,52 +122,67 @@ private function fillUpdatableFieldsWithCommandData(Product $product, UpdateProd $product->id_tax_rules_group = $taxRulesGroupId; $this->validateField($product, 'id_tax_rules_group', ProductConstraintException::INVALID_TAX_RULES_GROUP_ID); $this->assertTaxRulesGroupExists($taxRulesGroupId); - $this->fieldsToUpdate['id_tax_rules_group'] = true; + $updatableProperties[] = 'id_tax_rules_group'; } if (null !== $command->isOnSale()) { $product->on_sale = $command->isOnSale(); - $this->fieldsToUpdate['on_sale'] = true; + $updatableProperties[] = 'on_sale'; } if (null !== $command->getWholesalePrice()) { $product->wholesale_price = (string) $command->getWholesalePrice(); $this->validateField($product, 'wholesale_price', ProductConstraintException::INVALID_WHOLESALE_PRICE); - $this->fieldsToUpdate['wholesale_price'] = true; + $updatableProperties[] = 'wholesale_price'; } + + return $updatableProperties; } /** - * Update unit_price and unit_price ration depending on price - * - * @param UpdateProductPricesCommand $command - * - * @throws ProductConstraintException + * @param Product $product + * @param DecimalNumber $price + * @param DecimalNumber|null $unitPrice */ - private function handleUnitPriceInfo(UpdateProductPricesCommand $command): void + private function fillUnitPriceRatio(Product $product, DecimalNumber $price, ?DecimalNumber $unitPrice): void { - $price = $command->getPrice(); - $unitPrice = $command->getUnitPrice(); + // if price was reset then also reset unit_price_ratio + if ($price->equalsZero()) { + $this->setUnitPriceRatio($product, $price, $price); - // if price was reset then also reset unit_price and unit_price_ratio - $priceWasReset = null !== $price && $price->equalsZero(); - if ($priceWasReset) { - $this->resetUnitPriceInfo(); + return; } - //if price was not reset then allow setting new unit_price and unit_price_ratio - if (!$priceWasReset && null !== $unitPrice) { - $this->setUnitPriceInfo($this->product, $unitPrice, $price); + if (null === $unitPrice) { + $unitPrice = new DecimalNumber((string) $product->unit_price); } + + $this->setUnitPriceInfo($product, $unitPrice, $price); } /** + * @param Product $product + * @param DecimalNumber $price + * @param DecimalNumber $unitPrice + * * @throws ProductConstraintException + * @throws DivisionByZeroException */ - private function resetUnitPriceInfo(): void + private function setUnitPriceRatio(Product $product, DecimalNumber $price, DecimalNumber $unitPrice): void { - $zero = new DecimalNumber('0'); - $this->setUnitPriceInfo($this->product, $zero, $zero); + $this->validateUnitPrice($unitPrice); + + // If unit price or price is zero, then reset ratio to zero too + if ($unitPrice->equalsZero() || $price->equalsZero()) { + $ratio = new DecimalNumber('0'); + } else { + $ratio = $price->dividedBy($unitPrice); + } + + // unit_price_ratio is calculated based on input price and unit_price & then is saved to database, + // however - unit_price is not saved to database. When loading product it is calculated depending on price and unit_price_ratio + // so there is no static values saved, that's why unit_price is always inaccurate + $product->unit_price_ratio = (float) (string) $ratio; } /** @@ -209,23 +231,16 @@ private function setUnitPriceInfo(Product $product, DecimalNumber $unitPrice, ?D { $this->validateUnitPrice($unitPrice); - if (null === $price) { - $price = $this->numberExtractor->extract($product, 'price'); - } - // If unit price or price is zero, then reset ratio to zero too if ($unitPrice->equalsZero() || $price->equalsZero()) { $ratio = new DecimalNumber('0'); } else { $ratio = $price->dividedBy($unitPrice); } - + // unit_price_ratio is calculated based on input price and unit_price & then is saved to database, + // however - unit_price is not saved to database. When loading product it is calculated depending on price and unit_price_ratio + // so there is no static values saved, that's why unit_price is always inaccurate $product->unit_price_ratio = (float) (string) $ratio; - //unit_price is not saved to database, it is only calculated depending on price and unit_price_ratio - $product->unit_price = (float) (string) $unitPrice; - - $this->fieldsToUpdate['unit_price_ratio'] = true; - $this->fieldsToUpdate['unit_price'] = true; } /** diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 042aa770aee64..01df677471eb5 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -103,6 +103,7 @@ services: class: PrestaShop\PrestaShop\Adapter\Product\CommandHandler\UpdateProductPricesHandler arguments: - '@prestashop.core.util.number.number_extractor' + - '@prestashop.adapter.product.product_repository' tags: - name: tactician.handler command: PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand From 5981bae796d4a55777b8fb206489f6455fd1bc35 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 7 Oct 2020 12:23:07 +0300 Subject: [PATCH 02/18] move validations to Validator --- .../UpdateProductPricesHandler.php | 9 +-- .../Product/Validate/ProductValidator.php | 67 ++++++++++++------- 2 files changed, 45 insertions(+), 31 deletions(-) diff --git a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php index be7954195158d..38b53e49ac7bc 100644 --- a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php +++ b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php @@ -31,7 +31,6 @@ use PrestaShop\Decimal\DecimalNumber; use PrestaShop\Decimal\Exception\DivisionByZeroException; use PrestaShop\PrestaShop\Adapter\Entity\TaxRulesGroup; -use PrestaShop\PrestaShop\Adapter\Product\AbstractProductHandler; use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler\UpdateProductPricesHandlerInterface; @@ -46,7 +45,7 @@ /** * Updates product price information using legacy object models */ -final class UpdateProductPricesHandler extends AbstractProductHandler implements UpdateProductPricesHandlerInterface +final class UpdateProductPricesHandler implements UpdateProductPricesHandlerInterface { /** * @var NumberExtractor @@ -96,7 +95,6 @@ private function fillUpdatableProperties(Product $product, UpdateProductPricesCo $price = $command->getPrice(); if (null !== $price) { $product->price = (float) (string) $price; - $this->validateField($product, 'price', ProductConstraintException::INVALID_PRICE); $updatableProperties[] = 'price'; } else { $price = new DecimalNumber((string) $product->price); @@ -112,7 +110,6 @@ private function fillUpdatableProperties(Product $product, UpdateProductPricesCo if (null !== $command->getEcotax()) { $product->ecotax = (float) (string) $command->getEcotax(); - $this->validateField($product, 'ecotax', ProductConstraintException::INVALID_ECOTAX); $updatableProperties[] = 'ecotax'; } @@ -120,7 +117,6 @@ private function fillUpdatableProperties(Product $product, UpdateProductPricesCo if (null !== $taxRulesGroupId) { $product->id_tax_rules_group = $taxRulesGroupId; - $this->validateField($product, 'id_tax_rules_group', ProductConstraintException::INVALID_TAX_RULES_GROUP_ID); $this->assertTaxRulesGroupExists($taxRulesGroupId); $updatableProperties[] = 'id_tax_rules_group'; } @@ -131,8 +127,7 @@ private function fillUpdatableProperties(Product $product, UpdateProductPricesCo } if (null !== $command->getWholesalePrice()) { - $product->wholesale_price = (string) $command->getWholesalePrice(); - $this->validateField($product, 'wholesale_price', ProductConstraintException::INVALID_WHOLESALE_PRICE); + $product->wholesale_price = (float) (string) $command->getWholesalePrice(); $updatableProperties[] = 'wholesale_price'; } diff --git a/src/Adapter/Product/Validate/ProductValidator.php b/src/Adapter/Product/Validate/ProductValidator.php index d41dee871022a..83f3fdf620faa 100644 --- a/src/Adapter/Product/Validate/ProductValidator.php +++ b/src/Adapter/Product/Validate/ProductValidator.php @@ -31,6 +31,8 @@ use Pack; use PrestaShop\PrestaShop\Adapter\AbstractObjectModelValidator; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; +use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException as ProductConstraintExceptionAlias; +use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductException; use PrestaShop\PrestaShop\Core\Domain\Product\Pack\Exception\ProductPackConstraintException; use PrestaShop\PrestaShop\Core\Domain\Product\Stock\Exception\ProductStockConstraintException; use PrestaShop\PrestaShop\Core\Exception\CoreException; @@ -88,6 +90,8 @@ public function validateCreation(Product $product): void * @throws ProductConstraintException * @throws ProductPackConstraintException * @throws ProductStockConstraintException + * @throws ProductConstraintExceptionAlias + * @throws ProductException */ public function validate(Product $product): void { @@ -97,12 +101,13 @@ public function validate(Product $product): void $this->validateShipping($product); $this->validateStock($product); $this->validateSeo($product); + $this->validatePrices($product); } /** * @param Product $product * - * @throws ProductConstraintException + * @throws ProductConstraintExceptionAlias */ private function validateCustomizability(Product $product): void { @@ -114,13 +119,13 @@ private function validateCustomizability(Product $product): void /** * @param Product $product * - * @throws ProductConstraintException + * @throws ProductConstraintExceptionAlias */ private function validateBasicInfo(Product $product): void { - $this->validateProductLocalizedProperty($product, 'name', ProductConstraintException::INVALID_NAME); - $this->validateProductLocalizedProperty($product, 'description', ProductConstraintException::INVALID_DESCRIPTION); - $this->validateProductLocalizedProperty($product, 'description_short', ProductConstraintException::INVALID_SHORT_DESCRIPTION); + $this->validateProductLocalizedProperty($product, 'name', ProductConstraintExceptionAlias::INVALID_NAME); + $this->validateProductLocalizedProperty($product, 'description', ProductConstraintExceptionAlias::INVALID_DESCRIPTION); + $this->validateProductLocalizedProperty($product, 'description_short', ProductConstraintExceptionAlias::INVALID_SHORT_DESCRIPTION); } /** @@ -134,30 +139,44 @@ private function validateOptions(Product $product): void $this->validateProductProperty($product, 'online_only'); $this->validateProductProperty($product, 'show_price'); $this->validateProductProperty($product, 'id_manufacturer'); - $this->validateProductProperty($product, 'visibility', ProductConstraintException::INVALID_VISIBILITY); - $this->validateProductProperty($product, 'condition', ProductConstraintException::INVALID_CONDITION); - $this->validateProductProperty($product, 'ean13', ProductConstraintException::INVALID_EAN_13); - $this->validateProductProperty($product, 'isbn', ProductConstraintException::INVALID_ISBN); - $this->validateProductProperty($product, 'mpn', ProductConstraintException::INVALID_MPN); - $this->validateProductProperty($product, 'reference', ProductConstraintException::INVALID_REFERENCE); - $this->validateProductProperty($product, 'upc', ProductConstraintException::INVALID_UPC); + $this->validateProductProperty($product, 'visibility', ProductConstraintExceptionAlias::INVALID_VISIBILITY); + $this->validateProductProperty($product, 'condition', ProductConstraintExceptionAlias::INVALID_CONDITION); + $this->validateProductProperty($product, 'ean13', ProductConstraintExceptionAlias::INVALID_EAN_13); + $this->validateProductProperty($product, 'isbn', ProductConstraintExceptionAlias::INVALID_ISBN); + $this->validateProductProperty($product, 'mpn', ProductConstraintExceptionAlias::INVALID_MPN); + $this->validateProductProperty($product, 'reference', ProductConstraintExceptionAlias::INVALID_REFERENCE); + $this->validateProductProperty($product, 'upc', ProductConstraintExceptionAlias::INVALID_UPC); } /** * @param Product $product * - * @throws ProductConstraintException + * @throws ProductConstraintExceptionAlias */ private function validateShipping(Product $product): void { - $this->validateProductProperty($product, 'width', ProductConstraintException::INVALID_WIDTH); - $this->validateProductProperty($product, 'height', ProductConstraintException::INVALID_HEIGHT); - $this->validateProductProperty($product, 'depth', ProductConstraintException::INVALID_DEPTH); - $this->validateProductProperty($product, 'weight', ProductConstraintException::INVALID_WEIGHT); - $this->validateProductProperty($product, 'additional_shipping_cost', ProductConstraintException::INVALID_ADDITIONAL_SHIPPING_COST); + $this->validateProductProperty($product, 'width', ProductConstraintExceptionAlias::INVALID_WIDTH); + $this->validateProductProperty($product, 'height', ProductConstraintExceptionAlias::INVALID_HEIGHT); + $this->validateProductProperty($product, 'depth', ProductConstraintExceptionAlias::INVALID_DEPTH); + $this->validateProductProperty($product, 'weight', ProductConstraintExceptionAlias::INVALID_WEIGHT); + $this->validateProductProperty($product, 'additional_shipping_cost', ProductConstraintExceptionAlias::INVALID_ADDITIONAL_SHIPPING_COST); $this->validateProductProperty($product, 'additional_delivery_times'); - $this->validateProductLocalizedProperty($product, 'delivery_in_stock', ProductConstraintException::INVALID_DELIVERY_TIME_IN_STOCK_NOTES); - $this->validateProductLocalizedProperty($product, 'delivery_out_stock', ProductConstraintException::INVALID_DELIVERY_TIME_OUT_OF_STOCK_NOTES); + $this->validateProductLocalizedProperty($product, 'delivery_in_stock', ProductConstraintExceptionAlias::INVALID_DELIVERY_TIME_IN_STOCK_NOTES); + $this->validateProductLocalizedProperty($product, 'delivery_out_stock', ProductConstraintExceptionAlias::INVALID_DELIVERY_TIME_OUT_OF_STOCK_NOTES); + } + + /** + * @param Product $product + * + * @throws ProductConstraintExceptionAlias + */ + private function validatePrices(Product $product): void + { + $this->validateProductProperty($product, 'price', ProductConstraintExceptionAlias::INVALID_PRICE); + $this->validateProductProperty($product, 'unity'); + $this->validateProductProperty($product, 'ecotax', ProductConstraintExceptionAlias::INVALID_ECOTAX); + $this->validateProductProperty($product, 'id_tax_rules_group', ProductConstraintExceptionAlias::INVALID_TAX_RULES_GROUP_ID); + $this->validateProductProperty($product, 'wholesale_price', ProductConstraintExceptionAlias::INVALID_WHOLESALE_PRICE); } /** @@ -271,14 +290,14 @@ private function validateSeo(Product $product): void * @param string $propertyName * @param int $errorCode * - * @throws ProductConstraintException + * @throws ProductConstraintExceptionAlias */ private function validateProductProperty(Product $product, string $propertyName, int $errorCode = 0): void { $this->validateObjectModelProperty( $product, $propertyName, - ProductConstraintException::class, + ProductConstraintExceptionAlias::class, $errorCode ); } @@ -288,14 +307,14 @@ private function validateProductProperty(Product $product, string $propertyName, * @param string $propertyName * @param int $errorCode * - * @throws ProductConstraintException + * @throws ProductConstraintExceptionAlias */ private function validateProductLocalizedProperty(Product $product, string $propertyName, int $errorCode = 0): void { $this->validateObjectModelLocalizedProperty( $product, $propertyName, - ProductConstraintException::class, + ProductConstraintExceptionAlias::class, $errorCode ); } From dc845fa81fd1990d1b21e57b93b5d72c558aef27 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 7 Oct 2020 13:04:11 +0300 Subject: [PATCH 03/18] move taxRulesGroup assertion to corresponding repository & use in Validator. Adjust behats --- .../UpdateProductPricesHandler.php | 46 +--------- .../Product/Validate/ProductValidator.php | 85 ++++++++++++------- .../Repository/TaxRulesGroupRepository.php | 54 ++++++++++++ .../Exception/ProductConstraintException.php | 63 +++++++------- .../config/services/adapter/product.yml | 4 + .../services/adapter/tax_rules_group.yml | 3 + .../Product/CommonProductFeatureContext.php | 3 +- .../Product/UpdatePricesFeatureContext.php | 21 +++++ .../Domain/TaxRulesGroupFeatureContext.php | 9 ++ .../Scenario/Product/update_prices.feature | 2 +- 10 files changed, 178 insertions(+), 112 deletions(-) create mode 100644 src/Adapter/TaxRulesGroup/Repository/TaxRulesGroupRepository.php diff --git a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php index 38b53e49ac7bc..19662e440ea00 100644 --- a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php +++ b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php @@ -30,16 +30,12 @@ use PrestaShop\Decimal\DecimalNumber; use PrestaShop\Decimal\Exception\DivisionByZeroException; -use PrestaShop\PrestaShop\Adapter\Entity\TaxRulesGroup; use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler\UpdateProductPricesHandlerInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\CannotUpdateProductException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; -use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductException; -use PrestaShop\PrestaShop\Core\Domain\Product\ProductTaxRulesGroupSettings; use PrestaShop\PrestaShop\Core\Util\Number\NumberExtractor; -use PrestaShopException; use Product; /** @@ -117,7 +113,6 @@ private function fillUpdatableProperties(Product $product, UpdateProductPricesCo if (null !== $taxRulesGroupId) { $product->id_tax_rules_group = $taxRulesGroupId; - $this->assertTaxRulesGroupExists($taxRulesGroupId); $updatableProperties[] = 'id_tax_rules_group'; } @@ -180,41 +175,6 @@ private function setUnitPriceRatio(Product $product, DecimalNumber $price, Decim $product->unit_price_ratio = (float) (string) $ratio; } - /** - * @param int $taxRulesGroupId - * - * @throws ProductConstraintException - * @throws ProductException - */ - private function assertTaxRulesGroupExists(int $taxRulesGroupId): void - { - if (ProductTaxRulesGroupSettings::NONE_APPLIED === $taxRulesGroupId) { - return; - } - - try { - $taxRulesGroup = new TaxRulesGroup($taxRulesGroupId); - if (!$taxRulesGroup->id) { - throw new ProductConstraintException( - sprintf( - 'Invalid tax rules group id "%d". Group doesn\'t exist', - $taxRulesGroupId - ), - ProductConstraintException::INVALID_TAX_RULES_GROUP_ID - ); - } - } catch (PrestaShopException $e) { - throw new ProductException( - sprintf( - 'Error occurred when trying to load tax rules group #%d for product', - $taxRulesGroupId - ), - 0, - $e - ); - } - } - /** * @param Product $product * @param DecimalNumber $unitPrice @@ -224,8 +184,6 @@ private function assertTaxRulesGroupExists(int $taxRulesGroupId): void */ private function setUnitPriceInfo(Product $product, DecimalNumber $unitPrice, ?DecimalNumber $price): void { - $this->validateUnitPrice($unitPrice); - // If unit price or price is zero, then reset ratio to zero too if ($unitPrice->equalsZero() || $price->equalsZero()) { $ratio = new DecimalNumber('0'); @@ -234,8 +192,10 @@ private function setUnitPriceInfo(Product $product, DecimalNumber $unitPrice, ?D } // unit_price_ratio is calculated based on input price and unit_price & then is saved to database, // however - unit_price is not saved to database. When loading product it is calculated depending on price and unit_price_ratio - // so there is no static values saved, that's why unit_price is always inaccurate + // so there is no static values saved, that's why unit_price is inaccurate $product->unit_price_ratio = (float) (string) $ratio; + //set unit_price to go through validation + $product->unit_price = (float) (string) $unitPrice; } /** diff --git a/src/Adapter/Product/Validate/ProductValidator.php b/src/Adapter/Product/Validate/ProductValidator.php index 83f3fdf620faa..1d67a8d33c116 100644 --- a/src/Adapter/Product/Validate/ProductValidator.php +++ b/src/Adapter/Product/Validate/ProductValidator.php @@ -30,11 +30,13 @@ use Pack; use PrestaShop\PrestaShop\Adapter\AbstractObjectModelValidator; +use PrestaShop\PrestaShop\Adapter\TaxRulesGroup\Repository\TaxRulesGroupRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; -use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException as ProductConstraintExceptionAlias; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductException; use PrestaShop\PrestaShop\Core\Domain\Product\Pack\Exception\ProductPackConstraintException; +use PrestaShop\PrestaShop\Core\Domain\Product\ProductTaxRulesGroupSettings; use PrestaShop\PrestaShop\Core\Domain\Product\Stock\Exception\ProductStockConstraintException; +use PrestaShop\PrestaShop\Core\Domain\TaxRulesGroup\ValueObject\TaxRulesGroupId; use PrestaShop\PrestaShop\Core\Exception\CoreException; use Product; @@ -53,16 +55,24 @@ class ProductValidator extends AbstractObjectModelValidator */ private $defaultPackStockType; + /** + * @var TaxRulesGroupRepository + */ + private $taxRulesGroupRepository; + /** * @param bool $advancedStockEnabled * @param int $defaultPackStockType + * @param TaxRulesGroupRepository $taxRulesGroupRepository */ public function __construct( bool $advancedStockEnabled, - int $defaultPackStockType + int $defaultPackStockType, + TaxRulesGroupRepository $taxRulesGroupRepository ) { $this->advancedStockEnabled = $advancedStockEnabled; $this->defaultPackStockType = $defaultPackStockType; + $this->taxRulesGroupRepository = $taxRulesGroupRepository; } /** @@ -90,7 +100,7 @@ public function validateCreation(Product $product): void * @throws ProductConstraintException * @throws ProductPackConstraintException * @throws ProductStockConstraintException - * @throws ProductConstraintExceptionAlias + * @throws ProductConstraintException * @throws ProductException */ public function validate(Product $product): void @@ -107,7 +117,7 @@ public function validate(Product $product): void /** * @param Product $product * - * @throws ProductConstraintExceptionAlias + * @throws ProductConstraintException */ private function validateCustomizability(Product $product): void { @@ -119,13 +129,13 @@ private function validateCustomizability(Product $product): void /** * @param Product $product * - * @throws ProductConstraintExceptionAlias + * @throws ProductConstraintException */ private function validateBasicInfo(Product $product): void { - $this->validateProductLocalizedProperty($product, 'name', ProductConstraintExceptionAlias::INVALID_NAME); - $this->validateProductLocalizedProperty($product, 'description', ProductConstraintExceptionAlias::INVALID_DESCRIPTION); - $this->validateProductLocalizedProperty($product, 'description_short', ProductConstraintExceptionAlias::INVALID_SHORT_DESCRIPTION); + $this->validateProductLocalizedProperty($product, 'name', ProductConstraintException::INVALID_NAME); + $this->validateProductLocalizedProperty($product, 'description', ProductConstraintException::INVALID_DESCRIPTION); + $this->validateProductLocalizedProperty($product, 'description_short', ProductConstraintException::INVALID_SHORT_DESCRIPTION); } /** @@ -139,44 +149,55 @@ private function validateOptions(Product $product): void $this->validateProductProperty($product, 'online_only'); $this->validateProductProperty($product, 'show_price'); $this->validateProductProperty($product, 'id_manufacturer'); - $this->validateProductProperty($product, 'visibility', ProductConstraintExceptionAlias::INVALID_VISIBILITY); - $this->validateProductProperty($product, 'condition', ProductConstraintExceptionAlias::INVALID_CONDITION); - $this->validateProductProperty($product, 'ean13', ProductConstraintExceptionAlias::INVALID_EAN_13); - $this->validateProductProperty($product, 'isbn', ProductConstraintExceptionAlias::INVALID_ISBN); - $this->validateProductProperty($product, 'mpn', ProductConstraintExceptionAlias::INVALID_MPN); - $this->validateProductProperty($product, 'reference', ProductConstraintExceptionAlias::INVALID_REFERENCE); - $this->validateProductProperty($product, 'upc', ProductConstraintExceptionAlias::INVALID_UPC); + $this->validateProductProperty($product, 'visibility', ProductConstraintException::INVALID_VISIBILITY); + $this->validateProductProperty($product, 'condition', ProductConstraintException::INVALID_CONDITION); + $this->validateProductProperty($product, 'ean13', ProductConstraintException::INVALID_EAN_13); + $this->validateProductProperty($product, 'isbn', ProductConstraintException::INVALID_ISBN); + $this->validateProductProperty($product, 'mpn', ProductConstraintException::INVALID_MPN); + $this->validateProductProperty($product, 'reference', ProductConstraintException::INVALID_REFERENCE); + $this->validateProductProperty($product, 'upc', ProductConstraintException::INVALID_UPC); } /** * @param Product $product * - * @throws ProductConstraintExceptionAlias + * @throws ProductConstraintException */ private function validateShipping(Product $product): void { - $this->validateProductProperty($product, 'width', ProductConstraintExceptionAlias::INVALID_WIDTH); - $this->validateProductProperty($product, 'height', ProductConstraintExceptionAlias::INVALID_HEIGHT); - $this->validateProductProperty($product, 'depth', ProductConstraintExceptionAlias::INVALID_DEPTH); - $this->validateProductProperty($product, 'weight', ProductConstraintExceptionAlias::INVALID_WEIGHT); - $this->validateProductProperty($product, 'additional_shipping_cost', ProductConstraintExceptionAlias::INVALID_ADDITIONAL_SHIPPING_COST); + $this->validateProductProperty($product, 'width', ProductConstraintException::INVALID_WIDTH); + $this->validateProductProperty($product, 'height', ProductConstraintException::INVALID_HEIGHT); + $this->validateProductProperty($product, 'depth', ProductConstraintException::INVALID_DEPTH); + $this->validateProductProperty($product, 'weight', ProductConstraintException::INVALID_WEIGHT); + $this->validateProductProperty($product, 'additional_shipping_cost', ProductConstraintException::INVALID_ADDITIONAL_SHIPPING_COST); $this->validateProductProperty($product, 'additional_delivery_times'); - $this->validateProductLocalizedProperty($product, 'delivery_in_stock', ProductConstraintExceptionAlias::INVALID_DELIVERY_TIME_IN_STOCK_NOTES); - $this->validateProductLocalizedProperty($product, 'delivery_out_stock', ProductConstraintExceptionAlias::INVALID_DELIVERY_TIME_OUT_OF_STOCK_NOTES); + $this->validateProductLocalizedProperty($product, 'delivery_in_stock', ProductConstraintException::INVALID_DELIVERY_TIME_IN_STOCK_NOTES); + $this->validateProductLocalizedProperty($product, 'delivery_out_stock', ProductConstraintException::INVALID_DELIVERY_TIME_OUT_OF_STOCK_NOTES); } /** * @param Product $product * - * @throws ProductConstraintExceptionAlias + * @throws ProductConstraintException */ private function validatePrices(Product $product): void { - $this->validateProductProperty($product, 'price', ProductConstraintExceptionAlias::INVALID_PRICE); + $taxRulesGroupId = (int) $product->id_tax_rules_group; + if ($taxRulesGroupId !== ProductTaxRulesGroupSettings::NONE_APPLIED) { + $this->taxRulesGroupRepository->assertTaxRulesGroupExists(new TaxRulesGroupId($taxRulesGroupId)); + } + + if ($product->unit_price < 0) { + throw new ProductConstraintException( + sprintf('Invalid product unit_price. Got "%s"', $product->unit_price), + ProductConstraintException::INVALID_UNIT_PRICE + ); + } + + $this->validateProductProperty($product, 'price', ProductConstraintException::INVALID_PRICE); $this->validateProductProperty($product, 'unity'); - $this->validateProductProperty($product, 'ecotax', ProductConstraintExceptionAlias::INVALID_ECOTAX); - $this->validateProductProperty($product, 'id_tax_rules_group', ProductConstraintExceptionAlias::INVALID_TAX_RULES_GROUP_ID); - $this->validateProductProperty($product, 'wholesale_price', ProductConstraintExceptionAlias::INVALID_WHOLESALE_PRICE); + $this->validateProductProperty($product, 'ecotax', ProductConstraintException::INVALID_ECOTAX); + $this->validateProductProperty($product, 'wholesale_price', ProductConstraintException::INVALID_WHOLESALE_PRICE); } /** @@ -290,14 +311,14 @@ private function validateSeo(Product $product): void * @param string $propertyName * @param int $errorCode * - * @throws ProductConstraintExceptionAlias + * @throws ProductConstraintException */ private function validateProductProperty(Product $product, string $propertyName, int $errorCode = 0): void { $this->validateObjectModelProperty( $product, $propertyName, - ProductConstraintExceptionAlias::class, + ProductConstraintException::class, $errorCode ); } @@ -307,14 +328,14 @@ private function validateProductProperty(Product $product, string $propertyName, * @param string $propertyName * @param int $errorCode * - * @throws ProductConstraintExceptionAlias + * @throws ProductConstraintException */ private function validateProductLocalizedProperty(Product $product, string $propertyName, int $errorCode = 0): void { $this->validateObjectModelLocalizedProperty( $product, $propertyName, - ProductConstraintExceptionAlias::class, + ProductConstraintException::class, $errorCode ); } diff --git a/src/Adapter/TaxRulesGroup/Repository/TaxRulesGroupRepository.php b/src/Adapter/TaxRulesGroup/Repository/TaxRulesGroupRepository.php new file mode 100644 index 0000000000000..9a7b65ce533cd --- /dev/null +++ b/src/Adapter/TaxRulesGroup/Repository/TaxRulesGroupRepository.php @@ -0,0 +1,54 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\TaxRulesGroup\Repository; + +use PrestaShop\PrestaShop\Adapter\AbstractObjectModelRepository; +use PrestaShop\PrestaShop\Core\Domain\TaxRulesGroup\Exception\TaxRulesGroupNotFoundException; +use PrestaShop\PrestaShop\Core\Domain\TaxRulesGroup\ValueObject\TaxRulesGroupId; +use PrestaShop\PrestaShop\Core\Exception\CoreException; + +/** + * Provides access to TaxRulesGroup data source + */ +class TaxRulesGroupRepository extends AbstractObjectModelRepository +{ + /** + * @param TaxRulesGroupId $taxRulesGroupId + * + * @throws CoreException + */ + public function assertTaxRulesGroupExists(TaxRulesGroupId $taxRulesGroupId): void + { + $this->assertObjectModelExists( + $taxRulesGroupId->getValue(), + 'tax_rules_group', + TaxRulesGroupNotFoundException::class + ); + } +} diff --git a/src/Core/Domain/Product/Exception/ProductConstraintException.php b/src/Core/Domain/Product/Exception/ProductConstraintException.php index 17bf382c142ee..24ae67dcfe7a9 100644 --- a/src/Core/Domain/Product/Exception/ProductConstraintException.php +++ b/src/Core/Domain/Product/Exception/ProductConstraintException.php @@ -71,153 +71,148 @@ class ProductConstraintException extends ProductException */ const INVALID_ECOTAX = 80; - /** - * When invalid product tax rules group id is supplied - */ - const INVALID_TAX_RULES_GROUP_ID = 90; - /** * When invalid product unit price is supplied */ - const INVALID_UNIT_PRICE = 100; + const INVALID_UNIT_PRICE = 90; /** * When invalid product wholesale_price is supplied */ - const INVALID_WHOLESALE_PRICE = 110; + const INVALID_WHOLESALE_PRICE = 100; /** * When product visibility value is invalid */ - const INVALID_VISIBILITY = 120; + const INVALID_VISIBILITY = 110; /** * When product Ean13 code value is invalid */ - const INVALID_EAN_13 = 130; + const INVALID_EAN_13 = 120; /** * When product ISBN code value is invalid */ - const INVALID_ISBN = 140; + const INVALID_ISBN = 130; /** * When product mpn code value is invalid */ - const INVALID_MPN = 150; + const INVALID_MPN = 140; /** * When product upc code value is invalid */ - const INVALID_UPC = 160; + const INVALID_UPC = 150; /** * When product reference value is invalid */ - const INVALID_REFERENCE = 170; + const INVALID_REFERENCE = 160; /** * When product tag value is invalid */ - const INVALID_TAG = 180; + const INVALID_TAG = 170; /** * When product additional time notes type is invalid */ - const INVALID_ADDITIONAL_TIME_NOTES_TYPE = 190; + const INVALID_ADDITIONAL_TIME_NOTES_TYPE = 180; /** * When product width is invalid */ - const INVALID_WIDTH = 200; + const INVALID_WIDTH = 190; /** * When product height is invalid */ - const INVALID_HEIGHT = 210; + const INVALID_HEIGHT = 200; /** * When product depth is invalid */ - const INVALID_DEPTH = 220; + const INVALID_DEPTH = 210; /** * When product weight is invalid */ - const INVALID_WEIGHT = 230; + const INVALID_WEIGHT = 220; /** * When product additional shipping cost is invalid */ - const INVALID_ADDITIONAL_SHIPPING_COST = 240; + const INVALID_ADDITIONAL_SHIPPING_COST = 230; /** * When product delivery time in stock notes are invalid */ - const INVALID_DELIVERY_TIME_IN_STOCK_NOTES = 250; + const INVALID_DELIVERY_TIME_IN_STOCK_NOTES = 240; /** * When product delivery time out of stock notes are invalid */ - const INVALID_DELIVERY_TIME_OUT_OF_STOCK_NOTES = 260; + const INVALID_DELIVERY_TIME_OUT_OF_STOCK_NOTES = 250; /** * When product redirect type is invalid */ - const INVALID_REDIRECT_TYPE = 270; + const INVALID_REDIRECT_TYPE = 260; /** * When product redirect target */ - const INVALID_REDIRECT_TARGET = 280; + const INVALID_REDIRECT_TARGET = 270; /** * When product meta description is invalid */ - const INVALID_META_DESCRIPTION = 290; + const INVALID_META_DESCRIPTION = 280; /** * When product meta title is invalid */ - const INVALID_META_TITLE = 300; + const INVALID_META_TITLE = 290; /** * When product link rewrite is invalid */ - const INVALID_LINK_REWRITE = 310; + const INVALID_LINK_REWRITE = 300; /** * When product minimal quantity is invalid */ - const INVALID_MINIMAL_QUANTITY = 320; + const INVALID_MINIMAL_QUANTITY = 310; /** * When product location is invalid */ - const INVALID_LOCATION = 330; + const INVALID_LOCATION = 320; /** * When product available later labels are invalid */ - const INVALID_AVAILABLE_LATER = 340; + const INVALID_AVAILABLE_LATER = 330; /** * When product available now labels are is invalid */ - const INVALID_AVAILABLE_NOW = 350; + const INVALID_AVAILABLE_NOW = 340; /** * When product available date is invalid */ - const INVALID_AVAILABLE_DATE = 360; + const INVALID_AVAILABLE_DATE = 350; /** * When product low stock alert is invalid */ - const INVALID_LOW_STOCK_ALERT = 370; + const INVALID_LOW_STOCK_ALERT = 360; /** * When product low stock threshold is invalid */ - const INVALID_LOW_STOCK_THRESHOLD = 380; + const INVALID_LOW_STOCK_THRESHOLD = 370; } diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 01df677471eb5..2bc66a95c8a3b 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -284,8 +284,12 @@ services: prestashop.adapter.product.validate.product_validator: class: PrestaShop\PrestaShop\Adapter\Product\Validate\ProductValidator arguments: +<<<<<<< HEAD - '@=service("prestashop.adapter.legacy.configuration").getBoolean("PS_ADVANCED_STOCK_MANAGEMENT")' - '@=service("prestashop.adapter.legacy.configuration").getInt("PS_PACK_STOCK_TYPE")' +======= + - '@prestashop.adapter.tax_rules_group.repository.tax_rules_group_repository' +>>>>>>> move taxRulesGroup assertion to corresponding repository & use in Validator. Adjust behats prestashop.adapter.product.repository.product_repository: class: PrestaShop\PrestaShop\Adapter\Product\Repository\ProductRepository diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/tax_rules_group.yml b/src/PrestaShopBundle/Resources/config/services/adapter/tax_rules_group.yml index 6f8ef10330ef3..fd872c2553c23 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/tax_rules_group.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/tax_rules_group.yml @@ -31,3 +31,6 @@ services: tags: - name: 'tactician.handler' command: 'PrestaShop\PrestaShop\Core\Domain\TaxRulesGroup\Command\SetTaxRulesGroupStatusCommand' + + prestashop.adapter.tax_rules_group.repository.tax_rules_group_repository: + class: 'PrestaShop\PrestaShop\Adapter\TaxRulesGroup\Repository\TaxRulesGroupRepository' diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php index 2ce819f56d906..9fa13207c8a1a 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php @@ -31,8 +31,8 @@ use PHPUnit\Framework\Assert; use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; -use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductNotFoundException; +use PrestaShop\PrestaShop\Core\Exception\ProductException; use RuntimeException; use Tests\Integration\Behaviour\Features\Context\Util\CombinationDetails; use Tests\Integration\Behaviour\Features\Context\Util\ProductCombinationFactory; @@ -214,7 +214,6 @@ private function getConstraintErrorCode(string $fieldName): int 'ecotax' => ProductConstraintException::INVALID_ECOTAX, 'wholesale_price' => ProductConstraintException::INVALID_WHOLESALE_PRICE, 'unit_price' => ProductConstraintException::INVALID_UNIT_PRICE, - 'tax rules group' => ProductConstraintException::INVALID_TAX_RULES_GROUP_ID, 'tag' => ProductConstraintException::INVALID_TAG, 'width' => ProductConstraintException::INVALID_WIDTH, 'height' => ProductConstraintException::INVALID_HEIGHT, diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/UpdatePricesFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/UpdatePricesFeatureContext.php index 89dadd519b1b7..395c355d6eff0 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/UpdatePricesFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/UpdatePricesFeatureContext.php @@ -32,6 +32,7 @@ use Cache; use PHPUnit\Framework\Assert; use PrestaShop\Decimal\DecimalNumber; +use PrestaShop\PrestaShop\Core\Domain\Exception\DomainException; use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductException; use PrestaShop\PrestaShop\Core\Domain\Product\QueryResult\ProductPricesInformation; @@ -84,6 +85,26 @@ public function updateProductPrices(string $productReference, TableNode $table): } } + /** + * @When I update product :productReference prices and apply non-existing tax rules group + * + * @param string $productReference + */ + public function updateTaxRulesGroupWithNonExistingGroup(string $productReference): void + { + $productId = $this->getSharedStorage()->get($productReference); + + $command = new UpdateProductPricesCommand($productId); + // this id value does not exist, it is used on purpose. + $command->setTaxRulesGroupId(50000000); + + try { + $this->getCommandBus()->handle($command); + } catch (DomainException $e) { + $this->setLastException($e); + } + } + /** * @Then product :productReference should have following prices information: * diff --git a/tests/Integration/Behaviour/Features/Context/Domain/TaxRulesGroupFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/TaxRulesGroupFeatureContext.php index 8dda3a6e27b1f..97b8ad4e40fee 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/TaxRulesGroupFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/TaxRulesGroupFeatureContext.php @@ -28,6 +28,7 @@ namespace Tests\Integration\Behaviour\Features\Context\Domain; +use PrestaShop\PrestaShop\Core\Domain\TaxRulesGroup\Exception\TaxRulesGroupNotFoundException; use RuntimeException; use TaxRulesGroup; @@ -59,4 +60,12 @@ public function assertTaxRuleGroupExists(string $name) { self::getTaxRulesGroupByName($name); } + + /** + * @Then I should get error that tax rules group does not exist + */ + public function assertLastErrorIsTaxRulesGroupNotFound(): void + { + $this->assertLastErrorIs(TaxRulesGroupNotFoundException::class); + } } diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/update_prices.feature b/tests/Integration/Behaviour/Features/Scenario/Product/update_prices.feature index 1fa6d2cf79ddc..840563d3bc491 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/update_prices.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/update_prices.feature @@ -140,4 +140,4 @@ Feature: Update product price fields from Back Office (BO). And product product5 should have following prices information: | tax rules group | | When I update product "product5" prices and apply non-existing tax rules group - Then I should get error that product "tax rules group" is invalid + Then I should get error that tax rules group does not exist From 832c0b096618edc9d855e2a3d0131f95058d0a37 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 7 Oct 2020 15:17:25 +0300 Subject: [PATCH 04/18] POC PriceFiller service to wrap domain logic for update --- .../UpdateProductPricesHandler.php | 119 +++--------------- .../Product/Update/ProductPriceFiller.php | 105 ++++++++++++++++ .../Command/UpdateProductPricesCommand.php | 12 ++ .../config/services/adapter/product.yml | 4 + 4 files changed, 136 insertions(+), 104 deletions(-) create mode 100644 src/Adapter/Product/Update/ProductPriceFiller.php diff --git a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php index 19662e440ea00..7ad2ad6e33c0c 100644 --- a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php +++ b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php @@ -28,9 +28,8 @@ namespace PrestaShop\PrestaShop\Adapter\Product\CommandHandler; -use PrestaShop\Decimal\DecimalNumber; -use PrestaShop\Decimal\Exception\DivisionByZeroException; use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductRepository; +use PrestaShop\PrestaShop\Adapter\Product\Update\ProductPriceFiller; use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler\UpdateProductPricesHandlerInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\CannotUpdateProductException; @@ -53,16 +52,24 @@ final class UpdateProductPricesHandler implements UpdateProductPricesHandlerInte */ private $productRepository; + /** + * @var ProductPriceFiller + */ + private $productPriceFiller; + /** * @param NumberExtractor $numberExtractor * @param ProductRepository $productRepository + * @param ProductPriceFiller $productPriceFiller */ public function __construct( NumberExtractor $numberExtractor, - ProductRepository $productRepository + ProductRepository $productRepository, + ProductPriceFiller $productPriceFiller ) { $this->numberExtractor = $numberExtractor; $this->productRepository = $productRepository; + $this->productPriceFiller = $productPriceFiller; } /** @@ -86,18 +93,11 @@ public function handle(UpdateProductPricesCommand $command): void */ private function fillUpdatableProperties(Product $product, UpdateProductPricesCommand $command): array { - $updatableProperties = []; - - $price = $command->getPrice(); - if (null !== $price) { - $product->price = (float) (string) $price; - $updatableProperties[] = 'price'; - } else { - $price = new DecimalNumber((string) $product->price); - } - - $this->fillUnitPriceRatio($product, $price, $command->getUnitPrice()); - $updatableProperties[] = 'unit_price_ratio'; + $updatableProperties = $this->productPriceFiller->fillPrices( + $product, + $command->getPrice(), + $command->getUnitPrice() + ); if (null !== $command->getUnity()) { $product->unity = $command->getUnity(); @@ -128,93 +128,4 @@ private function fillUpdatableProperties(Product $product, UpdateProductPricesCo return $updatableProperties; } - - /** - * @param Product $product - * @param DecimalNumber $price - * @param DecimalNumber|null $unitPrice - */ - private function fillUnitPriceRatio(Product $product, DecimalNumber $price, ?DecimalNumber $unitPrice): void - { - // if price was reset then also reset unit_price_ratio - if ($price->equalsZero()) { - $this->setUnitPriceRatio($product, $price, $price); - - return; - } - - if (null === $unitPrice) { - $unitPrice = new DecimalNumber((string) $product->unit_price); - } - - $this->setUnitPriceInfo($product, $unitPrice, $price); - } - - /** - * @param Product $product - * @param DecimalNumber $price - * @param DecimalNumber $unitPrice - * - * @throws ProductConstraintException - * @throws DivisionByZeroException - */ - private function setUnitPriceRatio(Product $product, DecimalNumber $price, DecimalNumber $unitPrice): void - { - $this->validateUnitPrice($unitPrice); - - // If unit price or price is zero, then reset ratio to zero too - if ($unitPrice->equalsZero() || $price->equalsZero()) { - $ratio = new DecimalNumber('0'); - } else { - $ratio = $price->dividedBy($unitPrice); - } - - // unit_price_ratio is calculated based on input price and unit_price & then is saved to database, - // however - unit_price is not saved to database. When loading product it is calculated depending on price and unit_price_ratio - // so there is no static values saved, that's why unit_price is always inaccurate - $product->unit_price_ratio = (float) (string) $ratio; - } - - /** - * @param Product $product - * @param DecimalNumber $unitPrice - * @param DecimalNumber $price - * - * @throws ProductConstraintException - */ - private function setUnitPriceInfo(Product $product, DecimalNumber $unitPrice, ?DecimalNumber $price): void - { - // If unit price or price is zero, then reset ratio to zero too - if ($unitPrice->equalsZero() || $price->equalsZero()) { - $ratio = new DecimalNumber('0'); - } else { - $ratio = $price->dividedBy($unitPrice); - } - // unit_price_ratio is calculated based on input price and unit_price & then is saved to database, - // however - unit_price is not saved to database. When loading product it is calculated depending on price and unit_price_ratio - // so there is no static values saved, that's why unit_price is inaccurate - $product->unit_price_ratio = (float) (string) $ratio; - //set unit_price to go through validation - $product->unit_price = (float) (string) $unitPrice; - } - - /** - * Unit price validation is not involved in legacy validation, so it is checked manually to have unsigned int value - * - * @param DecimalNumber $unitPrice - * - * @throws ProductConstraintException - */ - private function validateUnitPrice(DecimalNumber $unitPrice): void - { - if ($unitPrice->isLowerThanZero()) { - throw new ProductConstraintException( - sprintf( - 'Invalid product unit_price. Got "%s"', - $unitPrice - ), - ProductConstraintException::INVALID_UNIT_PRICE - ); - } - } } diff --git a/src/Adapter/Product/Update/ProductPriceFiller.php b/src/Adapter/Product/Update/ProductPriceFiller.php new file mode 100644 index 0000000000000..a796f786345b0 --- /dev/null +++ b/src/Adapter/Product/Update/ProductPriceFiller.php @@ -0,0 +1,105 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Product\Update; + +use PrestaShop\Decimal\DecimalNumber; +use Product; + +/** + * Fills Product price values which needs specific handling for update + */ +class ProductPriceFiller +{ + /** + * Fills Product price & unit_price & unit_price_ratio which are depending on each other + * + * @param Product $product + * @param DecimalNumber|null $price + * @param DecimalNumber|null $unitPrice + * + * @return string[] updatable properties + */ + public function fillPrices(Product $product, ?DecimalNumber $price, ?DecimalNumber $unitPrice): array + { + if (null !== $price) { + $product->price = (float) (string) $price; + $updatableProperties[] = 'price'; + } else { + $price = new DecimalNumber((string) $product->price); + } + + $this->fillUnitPriceRatio($product, $price, $unitPrice); + $updatableProperties[] = 'unit_price_ratio'; + + return $updatableProperties; + } + + /** + * @param Product $product + * @param DecimalNumber $price + * @param DecimalNumber|null $unitPrice + */ + private function fillUnitPriceRatio(Product $product, DecimalNumber $price, ?DecimalNumber $unitPrice): void + { + // if price was reset then also reset unit_price_ratio + if ($price->equalsZero()) { + $this->setUnitPriceRatio($product, $price, $price); + + return; + } + + if (null === $unitPrice) { + $unitPrice = new DecimalNumber((string) $product->unit_price); + } + + // if price was not reset then allow setting new unit_price and unit_price_ratio + $this->setUnitPriceRatio($product, $price, $unitPrice); + } + + /** + * @param Product $product + * @param DecimalNumber $price + * @param DecimalNumber $unitPrice + */ + private function setUnitPriceRatio(Product $product, DecimalNumber $price, DecimalNumber $unitPrice): void + { + // If unit price or price is zero, then reset ratio to zero too + if ($unitPrice->equalsZero() || $price->equalsZero()) { + $ratio = new DecimalNumber('0'); + } else { + $ratio = $price->dividedBy($unitPrice); + } + // unit_price_ratio is calculated based on input price and unit_price & then is saved to database, + // however - unit_price is not saved to database. When loading product it is calculated depending on price and unit_price_ratio + // so there is no static values saved, that's why unit_price is inaccurate + $product->unit_price_ratio = (float) (string) $ratio; + //set unit_price to go through validation + $product->unit_price = (float) (string) $unitPrice; + } +} diff --git a/src/Core/Domain/Product/Command/UpdateProductPricesCommand.php b/src/Core/Domain/Product/Command/UpdateProductPricesCommand.php index 1ea2abe378df6..191fb55334b5f 100644 --- a/src/Core/Domain/Product/Command/UpdateProductPricesCommand.php +++ b/src/Core/Domain/Product/Command/UpdateProductPricesCommand.php @@ -93,7 +93,10 @@ public function getProductId(): ProductId } /** +<<<<<<< HEAD * @return DecimalNumber|null + * @return Number|null +>>>>>>> POC PriceFiller service to wrap domain logic for update */ public function getPrice(): ?DecimalNumber { @@ -113,7 +116,10 @@ public function setPrice(string $price): self } /** +<<<<<<< HEAD * @return DecimalNumber|null + * @return Number|null +>>>>>>> POC PriceFiller service to wrap domain logic for update */ public function getEcotax(): ?DecimalNumber { @@ -173,7 +179,10 @@ public function setOnSale(bool $onSale): self } /** +<<<<<<< HEAD * @return DecimalNumber|null + * @return Number|null +>>>>>>> POC PriceFiller service to wrap domain logic for update */ public function getWholesalePrice(): ?DecimalNumber { @@ -193,7 +202,10 @@ public function setWholesalePrice(string $wholesalePrice): self } /** +<<<<<<< HEAD * @return DecimalNumber|null + * @return Number|null +>>>>>>> POC PriceFiller service to wrap domain logic for update */ public function getUnitPrice(): ?DecimalNumber { diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 2bc66a95c8a3b..ad42351ac4bc4 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -104,6 +104,7 @@ services: arguments: - '@prestashop.core.util.number.number_extractor' - '@prestashop.adapter.product.product_repository' + - '@prestashop.adapter.product.product_price_filler' tags: - name: tactician.handler command: PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand @@ -299,6 +300,9 @@ services: - '@prestashop.adapter.product.validate.product_validator' - '@=service("prestashop.adapter.legacy.context").getContext().shop.id_category' + prestashop.adapter.product.product_price_filler: + class: PrestaShop\PrestaShop\Adapter\Product\Update\ProductPriceFiller + prestashop.adapter.product.validate.customization_field_validator: class: PrestaShop\PrestaShop\Adapter\Product\Validate\CustomizationFieldValidator From b7e422f755cd2f63f585fc1b28cf586e50d54f23 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 7 Oct 2020 15:37:00 +0300 Subject: [PATCH 05/18] fix param annotations to please phpstan --- .../Product/Command/UpdateProductPricesCommand.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/Core/Domain/Product/Command/UpdateProductPricesCommand.php b/src/Core/Domain/Product/Command/UpdateProductPricesCommand.php index 191fb55334b5f..1ea2abe378df6 100644 --- a/src/Core/Domain/Product/Command/UpdateProductPricesCommand.php +++ b/src/Core/Domain/Product/Command/UpdateProductPricesCommand.php @@ -93,10 +93,7 @@ public function getProductId(): ProductId } /** -<<<<<<< HEAD * @return DecimalNumber|null - * @return Number|null ->>>>>>> POC PriceFiller service to wrap domain logic for update */ public function getPrice(): ?DecimalNumber { @@ -116,10 +113,7 @@ public function setPrice(string $price): self } /** -<<<<<<< HEAD * @return DecimalNumber|null - * @return Number|null ->>>>>>> POC PriceFiller service to wrap domain logic for update */ public function getEcotax(): ?DecimalNumber { @@ -179,10 +173,7 @@ public function setOnSale(bool $onSale): self } /** -<<<<<<< HEAD * @return DecimalNumber|null - * @return Number|null ->>>>>>> POC PriceFiller service to wrap domain logic for update */ public function getWholesalePrice(): ?DecimalNumber { @@ -202,10 +193,7 @@ public function setWholesalePrice(string $wholesalePrice): self } /** -<<<<<<< HEAD * @return DecimalNumber|null - * @return Number|null ->>>>>>> POC PriceFiller service to wrap domain logic for update */ public function getUnitPrice(): ?DecimalNumber { From 903417ed50ede9562bcd62ffd71a5b178e8bed6b Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 12 Oct 2020 17:56:12 +0300 Subject: [PATCH 06/18] fix yml service name --- .../Resources/config/services/adapter/product.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index ad42351ac4bc4..90383d7b93072 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -103,7 +103,7 @@ services: class: PrestaShop\PrestaShop\Adapter\Product\CommandHandler\UpdateProductPricesHandler arguments: - '@prestashop.core.util.number.number_extractor' - - '@prestashop.adapter.product.product_repository' + - '@prestashop.adapter.product.repository.product_repository' - '@prestashop.adapter.product.product_price_filler' tags: - name: tactician.handler From 9c7f9d92e63cf912b38a7e052b74300752a6dc6b Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 12:54:46 +0200 Subject: [PATCH 07/18] rename ProductPriceProperties filler & method fillWithPrices. Add whole_sale price handling there too --- .../UpdateProductPricesHandler.php | 22 ++++++++----------- ...r.php => ProductPricePropertiesFiller.php} | 15 +++++++++---- .../config/services/adapter/product.yml | 4 ++-- 3 files changed, 22 insertions(+), 19 deletions(-) rename src/Adapter/Product/Update/{ProductPriceFiller.php => ProductPricePropertiesFiller.php} (83%) diff --git a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php index 7ad2ad6e33c0c..2abfd15cca786 100644 --- a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php +++ b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php @@ -29,7 +29,7 @@ namespace PrestaShop\PrestaShop\Adapter\Product\CommandHandler; use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductRepository; -use PrestaShop\PrestaShop\Adapter\Product\Update\ProductPriceFiller; +use PrestaShop\PrestaShop\Adapter\Product\Update\ProductPricePropertiesFiller; use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler\UpdateProductPricesHandlerInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\CannotUpdateProductException; @@ -53,23 +53,23 @@ final class UpdateProductPricesHandler implements UpdateProductPricesHandlerInte private $productRepository; /** - * @var ProductPriceFiller + * @var ProductPricePropertiesFiller */ - private $productPriceFiller; + private $productPricePropertiesFiller; /** * @param NumberExtractor $numberExtractor * @param ProductRepository $productRepository - * @param ProductPriceFiller $productPriceFiller + * @param ProductPricePropertiesFiller $productPricePropertiesFiller */ public function __construct( NumberExtractor $numberExtractor, ProductRepository $productRepository, - ProductPriceFiller $productPriceFiller + ProductPricePropertiesFiller $productPricePropertiesFiller ) { $this->numberExtractor = $numberExtractor; $this->productRepository = $productRepository; - $this->productPriceFiller = $productPriceFiller; + $this->productPricePropertiesFiller = $productPricePropertiesFiller; } /** @@ -93,10 +93,11 @@ public function handle(UpdateProductPricesCommand $command): void */ private function fillUpdatableProperties(Product $product, UpdateProductPricesCommand $command): array { - $updatableProperties = $this->productPriceFiller->fillPrices( + $updatableProperties = $this->productPricePropertiesFiller->fillWithPrices( $product, $command->getPrice(), - $command->getUnitPrice() + $command->getUnitPrice(), + $command->getWholesalePrice() ); if (null !== $command->getUnity()) { @@ -121,11 +122,6 @@ private function fillUpdatableProperties(Product $product, UpdateProductPricesCo $updatableProperties[] = 'on_sale'; } - if (null !== $command->getWholesalePrice()) { - $product->wholesale_price = (float) (string) $command->getWholesalePrice(); - $updatableProperties[] = 'wholesale_price'; - } - return $updatableProperties; } } diff --git a/src/Adapter/Product/Update/ProductPriceFiller.php b/src/Adapter/Product/Update/ProductPricePropertiesFiller.php similarity index 83% rename from src/Adapter/Product/Update/ProductPriceFiller.php rename to src/Adapter/Product/Update/ProductPricePropertiesFiller.php index a796f786345b0..89a57190f9406 100644 --- a/src/Adapter/Product/Update/ProductPriceFiller.php +++ b/src/Adapter/Product/Update/ProductPricePropertiesFiller.php @@ -32,21 +32,28 @@ use Product; /** - * Fills Product price values which needs specific handling for update + * Fills Product price properties which needs specific handling for update */ -class ProductPriceFiller +class ProductPricePropertiesFiller { /** - * Fills Product price & unit_price & unit_price_ratio which are depending on each other + * Wraps following properties filling: price, unit_price, unit_price_ratio, wholesale_price + * as most of them (price, unit_price, unit_price_ratio) are highly coupled & depends on each other * * @param Product $product * @param DecimalNumber|null $price * @param DecimalNumber|null $unitPrice + * @param DecimalNumber|null $wholesalePrice * * @return string[] updatable properties */ - public function fillPrices(Product $product, ?DecimalNumber $price, ?DecimalNumber $unitPrice): array + public function fillWithPrices(Product $product, ?DecimalNumber $price, ?DecimalNumber $unitPrice, ?DecimalNumber $wholesalePrice): array { + if (null !== $wholesalePrice) { + $product->wholesale_price = (float) (string) $wholesalePrice; + $updatableProperties[] = 'wholesale_price'; + } + if (null !== $price) { $product->price = (float) (string) $price; $updatableProperties[] = 'price'; diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 90383d7b93072..abf7e4121e1b3 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -300,8 +300,8 @@ services: - '@prestashop.adapter.product.validate.product_validator' - '@=service("prestashop.adapter.legacy.context").getContext().shop.id_category' - prestashop.adapter.product.product_price_filler: - class: PrestaShop\PrestaShop\Adapter\Product\Update\ProductPriceFiller + prestashop.adapter.product.product_price_properties_filler: + class: PrestaShop\PrestaShop\Adapter\Product\Update\ProductPricePropertiesFiller prestashop.adapter.product.validate.customization_field_validator: class: PrestaShop\PrestaShop\Adapter\Product\Validate\CustomizationFieldValidator From 0a13649de6b126163a13c5c84016a5489b4c7d82 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 13:30:29 +0200 Subject: [PATCH 08/18] move taxRulesGroup existence assertion to Repository --- .../Product/Repository/ProductRepository.php | 13 +++++++++++++ .../Product/Validate/ProductValidator.php | 18 +----------------- .../config/services/adapter/product.yml | 5 +---- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/Adapter/Product/Repository/ProductRepository.php b/src/Adapter/Product/Repository/ProductRepository.php index fc9c71a51c327..3366c86857ed9 100644 --- a/src/Adapter/Product/Repository/ProductRepository.php +++ b/src/Adapter/Product/Repository/ProductRepository.php @@ -32,6 +32,7 @@ use PrestaShop\Decimal\DecimalNumber; use PrestaShop\PrestaShop\Adapter\AbstractObjectModelRepository; use PrestaShop\PrestaShop\Adapter\Product\Validate\ProductValidator; +use PrestaShop\PrestaShop\Adapter\TaxRulesGroup\Repository\TaxRulesGroupRepository; use PrestaShop\PrestaShop\Core\Domain\Language\ValueObject\LanguageId; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\CannotAddProductException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\CannotBulkDeleteProductException; @@ -42,9 +43,11 @@ use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductNotFoundException; use PrestaShop\PrestaShop\Core\Domain\Product\Pack\Exception\ProductPackConstraintException; +use PrestaShop\PrestaShop\Core\Domain\Product\ProductTaxRulesGroupSettings; use PrestaShop\PrestaShop\Core\Domain\Product\Stock\Exception\ProductStockConstraintException; use PrestaShop\PrestaShop\Core\Domain\Product\ValueObject\ProductId; use PrestaShop\PrestaShop\Core\Domain\Shop\ValueObject\ShopId; +use PrestaShop\PrestaShop\Core\Domain\TaxRulesGroup\ValueObject\TaxRulesGroupId; use PrestaShop\PrestaShop\Core\Exception\CoreException; use PrestaShopException; use Product; @@ -74,6 +77,11 @@ class ProductRepository extends AbstractObjectModelRepository */ private $defaultCategoryId; + /** + * @var TaxRulesGroupRepository + */ + private $taxRulesGroupRepository; + /** * @param Connection $connection * @param string $dbPrefix @@ -275,6 +283,11 @@ public function create(array $localizedNames, bool $isVirtual): Product */ public function partialUpdate(Product $product, array $propertiesToUpdate, int $errorCode): void { + $taxRulesGroupId = (int) $product->id_tax_rules_group; + if ($taxRulesGroupId !== ProductTaxRulesGroupSettings::NONE_APPLIED) { + $this->taxRulesGroupRepository->assertTaxRulesGroupExists(new TaxRulesGroupId($taxRulesGroupId)); + } + $this->productValidator->validate($product); $this->partiallyUpdateObjectModel( $product, diff --git a/src/Adapter/Product/Validate/ProductValidator.php b/src/Adapter/Product/Validate/ProductValidator.php index 1d67a8d33c116..567bdc370ed92 100644 --- a/src/Adapter/Product/Validate/ProductValidator.php +++ b/src/Adapter/Product/Validate/ProductValidator.php @@ -30,13 +30,10 @@ use Pack; use PrestaShop\PrestaShop\Adapter\AbstractObjectModelValidator; -use PrestaShop\PrestaShop\Adapter\TaxRulesGroup\Repository\TaxRulesGroupRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductException; use PrestaShop\PrestaShop\Core\Domain\Product\Pack\Exception\ProductPackConstraintException; -use PrestaShop\PrestaShop\Core\Domain\Product\ProductTaxRulesGroupSettings; use PrestaShop\PrestaShop\Core\Domain\Product\Stock\Exception\ProductStockConstraintException; -use PrestaShop\PrestaShop\Core\Domain\TaxRulesGroup\ValueObject\TaxRulesGroupId; use PrestaShop\PrestaShop\Core\Exception\CoreException; use Product; @@ -55,24 +52,16 @@ class ProductValidator extends AbstractObjectModelValidator */ private $defaultPackStockType; - /** - * @var TaxRulesGroupRepository - */ - private $taxRulesGroupRepository; - /** * @param bool $advancedStockEnabled * @param int $defaultPackStockType - * @param TaxRulesGroupRepository $taxRulesGroupRepository */ public function __construct( bool $advancedStockEnabled, - int $defaultPackStockType, - TaxRulesGroupRepository $taxRulesGroupRepository + int $defaultPackStockType ) { $this->advancedStockEnabled = $advancedStockEnabled; $this->defaultPackStockType = $defaultPackStockType; - $this->taxRulesGroupRepository = $taxRulesGroupRepository; } /** @@ -182,11 +171,6 @@ private function validateShipping(Product $product): void */ private function validatePrices(Product $product): void { - $taxRulesGroupId = (int) $product->id_tax_rules_group; - if ($taxRulesGroupId !== ProductTaxRulesGroupSettings::NONE_APPLIED) { - $this->taxRulesGroupRepository->assertTaxRulesGroupExists(new TaxRulesGroupId($taxRulesGroupId)); - } - if ($product->unit_price < 0) { throw new ProductConstraintException( sprintf('Invalid product unit_price. Got "%s"', $product->unit_price), diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index abf7e4121e1b3..33bc01ce47eb5 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -285,12 +285,8 @@ services: prestashop.adapter.product.validate.product_validator: class: PrestaShop\PrestaShop\Adapter\Product\Validate\ProductValidator arguments: -<<<<<<< HEAD - '@=service("prestashop.adapter.legacy.configuration").getBoolean("PS_ADVANCED_STOCK_MANAGEMENT")' - '@=service("prestashop.adapter.legacy.configuration").getInt("PS_PACK_STOCK_TYPE")' -======= - - '@prestashop.adapter.tax_rules_group.repository.tax_rules_group_repository' ->>>>>>> move taxRulesGroup assertion to corresponding repository & use in Validator. Adjust behats prestashop.adapter.product.repository.product_repository: class: PrestaShop\PrestaShop\Adapter\Product\Repository\ProductRepository @@ -299,6 +295,7 @@ services: - '%database_prefix%' - '@prestashop.adapter.product.validate.product_validator' - '@=service("prestashop.adapter.legacy.context").getContext().shop.id_category' + - '@prestashop.adapter.tax_rules_group.repository.tax_rules_group_repository' prestashop.adapter.product.product_price_properties_filler: class: PrestaShop\PrestaShop\Adapter\Product\Update\ProductPricePropertiesFiller From 9704f982c813f24f21cfd66e067ba5ea00a72b2f Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 14:48:11 +0200 Subject: [PATCH 09/18] remove float cast from wholesale price --- src/Adapter/Product/Update/ProductPricePropertiesFiller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Product/Update/ProductPricePropertiesFiller.php b/src/Adapter/Product/Update/ProductPricePropertiesFiller.php index 89a57190f9406..2688dfe1ae610 100644 --- a/src/Adapter/Product/Update/ProductPricePropertiesFiller.php +++ b/src/Adapter/Product/Update/ProductPricePropertiesFiller.php @@ -50,7 +50,7 @@ class ProductPricePropertiesFiller public function fillWithPrices(Product $product, ?DecimalNumber $price, ?DecimalNumber $unitPrice, ?DecimalNumber $wholesalePrice): array { if (null !== $wholesalePrice) { - $product->wholesale_price = (float) (string) $wholesalePrice; + $product->wholesale_price = (string) $wholesalePrice; $updatableProperties[] = 'wholesale_price'; } From 7ebe1901fcd283e197933cbc8dd7b872857a71cf Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 14:49:59 +0200 Subject: [PATCH 10/18] fix service name in arguments --- .../Resources/config/services/adapter/product.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 33bc01ce47eb5..1fcad99b2d79d 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -104,7 +104,7 @@ services: arguments: - '@prestashop.core.util.number.number_extractor' - '@prestashop.adapter.product.repository.product_repository' - - '@prestashop.adapter.product.product_price_filler' + - '@prestashop.adapter.product.product_price_properties_filler' tags: - name: tactician.handler command: PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand From 02e61d511ebf4df9d7e698fdd02670de13a37e99 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 17:11:33 +0200 Subject: [PATCH 11/18] fix constructor argument --- src/Adapter/Product/Repository/ProductRepository.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Adapter/Product/Repository/ProductRepository.php b/src/Adapter/Product/Repository/ProductRepository.php index 3366c86857ed9..c5eeee144df51 100644 --- a/src/Adapter/Product/Repository/ProductRepository.php +++ b/src/Adapter/Product/Repository/ProductRepository.php @@ -87,17 +87,20 @@ class ProductRepository extends AbstractObjectModelRepository * @param string $dbPrefix * @param ProductValidator $productValidator * @param int $defaultCategoryId + * @param TaxRulesGroupRepository $taxRulesGroupRepository */ public function __construct( Connection $connection, string $dbPrefix, ProductValidator $productValidator, - int $defaultCategoryId + int $defaultCategoryId, + TaxRulesGroupRepository $taxRulesGroupRepository ) { $this->connection = $connection; $this->dbPrefix = $dbPrefix; $this->productValidator = $productValidator; $this->defaultCategoryId = $defaultCategoryId; + $this->taxRulesGroupRepository = $taxRulesGroupRepository; } /** From 68c54708fa765087c6d379dc38d2e6a7297f0d4d Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 17:16:15 +0200 Subject: [PATCH 12/18] remove duplicated context method --- .../Product/CommonProductFeatureContext.php | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php index 9fa13207c8a1a..bebd8a637166f 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php @@ -138,26 +138,6 @@ public function assertProductDoesNotExistAnymore(string $reference) } } - /** - * @When I update product :productReference prices and apply non-existing tax rules group - * - * @param string $productReference - */ - public function updateTaxRulesGroupWithNonExistingGroup(string $productReference): void - { - $productId = $this->getSharedStorage()->get($productReference); - - $command = new UpdateProductPricesCommand($productId); - // this id value does not exist, it is used on purpose. - $command->setTaxRulesGroupId(50000000); - - try { - $this->getCommandBus()->handle($command); - } catch (ProductException $e) { - $this->setLastException($e); - } - } - /** * @Then product :productReference type should be :productType * From 084be918977617f5d6c3d02a2407b95f6bdb0efb Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 17:35:51 +0200 Subject: [PATCH 13/18] save combination by reference instead of re-adding it over and over again --- .../Product/CommonProductFeatureContext.php | 15 ++++++------ .../Context/Util/CombinationDetails.php | 23 +++++++++++++++++-- .../Util/ProductCombinationFactory.php | 3 ++- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php index bebd8a637166f..2fa59b4c00b10 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php @@ -29,10 +29,8 @@ use Behat\Gherkin\Node\TableNode; use Language; use PHPUnit\Framework\Assert; -use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductNotFoundException; -use PrestaShop\PrestaShop\Core\Exception\ProductException; use RuntimeException; use Tests\Integration\Behaviour\Features\Context\Util\CombinationDetails; use Tests\Integration\Behaviour\Features\Context\Util\ProductCombinationFactory; @@ -48,20 +46,21 @@ class CommonProductFeatureContext extends AbstractProductFeatureContext public function addCombinationsToProduct(string $productReference, TableNode $tableNode) { $details = $tableNode->getColumnsHash(); + $productId = $this->getSharedStorage()->get($productReference); $combinationsDetails = []; foreach ($details as $combination) { + $combinationReference = $combination['reference']; + $combinationsDetails[] = new CombinationDetails( - $combination['reference'], + $combinationReference, (int) $combination['quantity'], - explode(';', $combination['attributes']) + explode(';', $combination['attributes']), + $this->getSharedStorage()->exists($combinationReference) ? $this->getSharedStorage()->get($combinationReference) : null ); } - $combinations = ProductCombinationFactory::makeCombinations( - $this->getSharedStorage()->get($productReference), - $combinationsDetails - ); + $combinations = ProductCombinationFactory::makeCombinations($productId, $combinationsDetails); foreach ($combinations as $combination) { $this->getSharedStorage()->set($combination->reference, (int) $combination->id); diff --git a/tests/Integration/Behaviour/Features/Context/Util/CombinationDetails.php b/tests/Integration/Behaviour/Features/Context/Util/CombinationDetails.php index 22a129be153ad..63fc4e1de1904 100644 --- a/tests/Integration/Behaviour/Features/Context/Util/CombinationDetails.php +++ b/tests/Integration/Behaviour/Features/Context/Util/CombinationDetails.php @@ -48,16 +48,27 @@ class CombinationDetails */ private $attributes; + /** + * @var int|null + */ + private $combinationId; + /** * @param string $reference * @param int $quantity * @param string[] $attributes + * @param int|null $combinationId to indicate update action instead of create */ - public function __construct(string $reference, int $quantity, array $attributes) - { + public function __construct( + string $reference, + int $quantity, + array $attributes, + ?int $combinationId = null + ) { $this->reference = $reference; $this->quantity = $quantity; $this->attributes = $attributes; + $this->combinationId = $combinationId; } /** @@ -83,4 +94,12 @@ public function getAttributes(): array { return $this->attributes; } + + /** + * @return int|null + */ + public function getCombinationId(): ?int + { + return $this->combinationId; + } } diff --git a/tests/Integration/Behaviour/Features/Context/Util/ProductCombinationFactory.php b/tests/Integration/Behaviour/Features/Context/Util/ProductCombinationFactory.php index 576f102e8b30a..f26c1b0a5da9c 100644 --- a/tests/Integration/Behaviour/Features/Context/Util/ProductCombinationFactory.php +++ b/tests/Integration/Behaviour/Features/Context/Util/ProductCombinationFactory.php @@ -54,10 +54,11 @@ public static function makeCombinations(int $productId, array $combinationDetail foreach ($combinationDetailsList as $combinationDetails) { $combinationName = $combinationDetails->getReference(); $combination = new Combination(); + $combination->id = $combinationDetails->getCombinationId(); $combination->reference = $combinationName; $combination->id_product = $productId; $combination->quantity = $combinationDetails->getQuantity(); - $combination->add(); + $combination->save(); StockAvailable::setQuantity($productId, $combination->id, (int) $combination->quantity); $combinations[] = $combination; $combinationAttributesIds = []; From 9a5849e7c65519ba9f077e77157fe22f6982a02d Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 17:47:17 +0200 Subject: [PATCH 14/18] Revert " save combination by reference instead of re-adding it over and over again" This reverts commit 46cdf9bdd066e493eb32446118a64ed98be677ea. --- .../Product/CommonProductFeatureContext.php | 15 ++++++------ .../Context/Util/CombinationDetails.php | 23 ++----------------- .../Util/ProductCombinationFactory.php | 3 +-- 3 files changed, 11 insertions(+), 30 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php index 2fa59b4c00b10..bebd8a637166f 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php @@ -29,8 +29,10 @@ use Behat\Gherkin\Node\TableNode; use Language; use PHPUnit\Framework\Assert; +use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductNotFoundException; +use PrestaShop\PrestaShop\Core\Exception\ProductException; use RuntimeException; use Tests\Integration\Behaviour\Features\Context\Util\CombinationDetails; use Tests\Integration\Behaviour\Features\Context\Util\ProductCombinationFactory; @@ -46,21 +48,20 @@ class CommonProductFeatureContext extends AbstractProductFeatureContext public function addCombinationsToProduct(string $productReference, TableNode $tableNode) { $details = $tableNode->getColumnsHash(); - $productId = $this->getSharedStorage()->get($productReference); $combinationsDetails = []; foreach ($details as $combination) { - $combinationReference = $combination['reference']; - $combinationsDetails[] = new CombinationDetails( - $combinationReference, + $combination['reference'], (int) $combination['quantity'], - explode(';', $combination['attributes']), - $this->getSharedStorage()->exists($combinationReference) ? $this->getSharedStorage()->get($combinationReference) : null + explode(';', $combination['attributes']) ); } - $combinations = ProductCombinationFactory::makeCombinations($productId, $combinationsDetails); + $combinations = ProductCombinationFactory::makeCombinations( + $this->getSharedStorage()->get($productReference), + $combinationsDetails + ); foreach ($combinations as $combination) { $this->getSharedStorage()->set($combination->reference, (int) $combination->id); diff --git a/tests/Integration/Behaviour/Features/Context/Util/CombinationDetails.php b/tests/Integration/Behaviour/Features/Context/Util/CombinationDetails.php index 63fc4e1de1904..22a129be153ad 100644 --- a/tests/Integration/Behaviour/Features/Context/Util/CombinationDetails.php +++ b/tests/Integration/Behaviour/Features/Context/Util/CombinationDetails.php @@ -48,27 +48,16 @@ class CombinationDetails */ private $attributes; - /** - * @var int|null - */ - private $combinationId; - /** * @param string $reference * @param int $quantity * @param string[] $attributes - * @param int|null $combinationId to indicate update action instead of create */ - public function __construct( - string $reference, - int $quantity, - array $attributes, - ?int $combinationId = null - ) { + public function __construct(string $reference, int $quantity, array $attributes) + { $this->reference = $reference; $this->quantity = $quantity; $this->attributes = $attributes; - $this->combinationId = $combinationId; } /** @@ -94,12 +83,4 @@ public function getAttributes(): array { return $this->attributes; } - - /** - * @return int|null - */ - public function getCombinationId(): ?int - { - return $this->combinationId; - } } diff --git a/tests/Integration/Behaviour/Features/Context/Util/ProductCombinationFactory.php b/tests/Integration/Behaviour/Features/Context/Util/ProductCombinationFactory.php index f26c1b0a5da9c..576f102e8b30a 100644 --- a/tests/Integration/Behaviour/Features/Context/Util/ProductCombinationFactory.php +++ b/tests/Integration/Behaviour/Features/Context/Util/ProductCombinationFactory.php @@ -54,11 +54,10 @@ public static function makeCombinations(int $productId, array $combinationDetail foreach ($combinationDetailsList as $combinationDetails) { $combinationName = $combinationDetails->getReference(); $combination = new Combination(); - $combination->id = $combinationDetails->getCombinationId(); $combination->reference = $combinationName; $combination->id_product = $productId; $combination->quantity = $combinationDetails->getQuantity(); - $combination->save(); + $combination->add(); StockAvailable::setQuantity($productId, $combination->id, (int) $combination->quantity); $combinations[] = $combination; $combinationAttributesIds = []; From b775b248648161247b3490c5579000247b1cc2ad Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 17:51:01 +0200 Subject: [PATCH 15/18] move product creation in behat background --- .../Product/CommonProductFeatureContext.php | 2 - .../Scenario/Product/update_prices.feature | 262 ++++++++++-------- 2 files changed, 143 insertions(+), 121 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php index bebd8a637166f..4cb4b4c900011 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php @@ -29,10 +29,8 @@ use Behat\Gherkin\Node\TableNode; use Language; use PHPUnit\Framework\Assert; -use PrestaShop\PrestaShop\Core\Domain\Product\Command\UpdateProductPricesCommand; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductNotFoundException; -use PrestaShop\PrestaShop\Core\Exception\ProductException; use RuntimeException; use Tests\Integration\Behaviour\Features\Context\Util\CombinationDetails; use Tests\Integration\Behaviour\Features\Context\Util\ProductCombinationFactory; diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/update_prices.feature b/tests/Integration/Behaviour/Features/Scenario/Product/update_prices.feature index 840563d3bc491..cb0400f79f058 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/update_prices.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/update_prices.feature @@ -4,140 +4,164 @@ Feature: Update product price fields from Back Office (BO). As a BO user I want to be able to update product fields associated with price. - Scenario: I update product prices + Background: Given I add product "product1" with following information: - | name | en-US:magic staff | - | is_virtual | false | + | name | en-US:magic staff | + | is_virtual | false | And product product1 should have following prices information: - | price | 0 | - | ecotax | 0 | - | tax rules group | | - | on_sale | false | - | wholesale_price | 0 | - | unit_price | 0 | - | unity | | - | unit_price_ratio | 0 | + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 0 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | + + Scenario: I update product prices And tax rules group named "US-AL Rate (4%)" exists When I update product "product1" prices with following information: - | price | 100.99 | - | ecotax | 0 | - | tax rules group | US-AL Rate (4%) | - | on_sale | true | - | wholesale_price | 70 | - | unit_price | 900 | - | unity | bag of ten | + | price | 100.99 | + | ecotax | 0 | + | tax rules group | US-AL Rate (4%) | + | on_sale | true | + | wholesale_price | 70 | + | unit_price | 900 | + | unity | bag of ten | Then product product1 should have following prices information: - | price | 100.99 | - | ecotax | 0 | - | tax rules group | US-AL Rate (4%) | - | on_sale | true | - | wholesale_price | 70 | + | price | 100.99 | + | ecotax | 0 | + | tax rules group | US-AL Rate (4%) | + | on_sale | true | + | wholesale_price | 70 | # #todo: rounding issue. #19620 # #| unit_price | 900 | - | unity | bag of ten | - | unit_price_ratio | 0.112211 | + | unity | bag of ten | + | unit_price_ratio | 0.112211 | - Scenario: I partially update product prices, providing only those values which I want to update - Given product product1 should have following prices information: - | price | 100.99 | - | ecotax | 0 | - | tax rules group | US-AL Rate (4%) | - | on_sale | true | - | wholesale_price | 70 | + Scenario: I partially update product prices, providing only those values which I want to update + Given I update product "product1" prices with following information: + | price | 100.99 | + | ecotax | 0 | + | tax rules group | US-AL Rate (4%) | + | on_sale | true | + | wholesale_price | 70 | + | unit_price | 900 | + | unity | bag of ten | + Given product product1 should have following prices information: + | price | 100.99 | + | ecotax | 0 | + | tax rules group | US-AL Rate (4%) | + | on_sale | true | + | wholesale_price | 70 | # #todo: rounding issue. #19620 # #| unit_price | 900 | - | unity | bag of ten | - | unit_price_ratio | 0.112211 | - When I update product "product1" prices with following information: - | price | 200 | - Then product product1 should have following prices information: - | price | 200 | - | ecotax | 0 | - | tax rules group | US-AL Rate (4%) | - | on_sale | true | - | wholesale_price | 70 | + | unity | bag of ten | + | unit_price_ratio | 0.112211 | + When I update product "product1" prices with following information: + | price | 200 | + Then product product1 should have following prices information: + | price | 200 | + | ecotax | 0 | + | tax rules group | US-AL Rate (4%) | + | on_sale | true | + | wholesale_price | 70 | # #todo: rounding issue. #19620 # #| unit_price | 900 | - | unity | bag of ten | - | unit_price_ratio | 0.222222 | - When I update product "product1" prices with following information: - | ecotax | 5.5 | - | on_sale | false | - Then product product1 should have following prices information: - | price | 200 | - | ecotax | 5.5 | - | tax rules group | US-AL Rate (4%) | - | on_sale | false | - | wholesale_price | 70 | + | unity | bag of ten | + | unit_price_ratio | 0.222222 | + When I update product "product1" prices with following information: + | ecotax | 5.5 | + | on_sale | false | + Then product product1 should have following prices information: + | price | 200 | + | ecotax | 5.5 | + | tax rules group | US-AL Rate (4%) | + | on_sale | false | + | wholesale_price | 70 | # #todo: rounding issue. #19620 # #| unit_price | 900 | - | unity | bag of ten | - | unit_price_ratio | 0.222222 | + | unity | bag of ten | + | unit_price_ratio | 0.222222 | - Scenario: I update product prices with negative values - Given I add product "product2" with following information: - | name | en-US: white hat | - | is_virtual | false | - And I update product "product2" prices with following information: - | price | 50 | - And product product2 should have following prices information: - | price | 50 | - | ecotax | 0 | - | wholesale_price | 0 | - | unit_price | 0 | - When I update product "product2" prices with following information: - | price | -20 | - Then I should get error that product "price" is invalid - When I update product "product2" prices with following information: - | ecotax | -2 | - Then I should get error that product "ecotax" is invalid - When I update product "product2" prices with following information: - | wholesale_price | -35 | - Then I should get error that product "wholesale_price" is invalid - When I update product "product2" prices with following information: - | unit_price | -300 | - Then I should get error that product "unit_price" is invalid + Scenario: I update product prices with negative values + Given I update product "product1" prices with following information: + | price | 50 | + | ecotax | 3 | + | tax rules group | US-AL Rate (4%) | + | on_sale | true | + | wholesale_price | 10 | + | unit_price | 500 | + | unity | bag of ten | + And product product1 should have following prices information: + | price | 50 | + | ecotax | 3 | + | tax rules group | US-AL Rate (4%) | + | on_sale | true | + | wholesale_price | 10 | + | unit_price | 500 | + | unity | bag of ten | + | unit_price_ratio | 0.1 | + When I update product "product1" prices with following information: + | price | -20 | + Then I should get error that product "price" is invalid + When I update product "product1" prices with following information: + | ecotax | -2 | + Then I should get error that product "ecotax" is invalid + When I update product "product1" prices with following information: + | wholesale_price | -35 | + Then I should get error that product "wholesale_price" is invalid + When I update product "product1" prices with following information: + | unit_price | -300 | + Then I should get error that product "unit_price" is invalid + And product product1 should have following prices information: + | price | 50 | + | ecotax | 3 | + | tax rules group | US-AL Rate (4%) | + | on_sale | true | + | wholesale_price | 10 | + | unit_price | 500 | + | unity | bag of ten | + | unit_price_ratio | 0.1 | - Scenario: I update product unit price when product price is 0 - Given I add product "product3" with following information: - | name | en-US: black hat | - | is_virtual | false | - And product product3 should have following prices information: - | price | 0 | - | unit_price | 0 | - When I update product "product3" prices with following information: - | unit_price | 300 | - Then product product3 should have following prices information: - | price | 0 | - | unit_price | 0 | + Scenario: I update product unit price when product price is 0 + When I update product "product1" prices with following information: + | unit_price | 300 | + Then product product1 should have following prices information: + | price | 0 | + | unit_price | 0 | + And product product1 should have following prices information: + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 0 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | - Scenario: I update product unit price along with product price - Given I add product "product4" with following information: - | name | en-US: blue dress | - | is_virtual | false | - And product product4 should have following prices information: - | price | 0 | - | unit_price | 0 | - When I update product "product4" prices with following information: - | price | 20 | - | unit_price | 500 | - Then product product4 should have following prices information: - | price | 20 | - | unit_price | 500 | - | unit_price_ratio | 0.04 | - When I update product "product4" prices with following information: - | price | 0 | - | unit_price | 500 | - Then product product4 should have following prices information: - | price | 0 | - | unit_price | 0 | - | unit_price_ratio | 0 | + Scenario: I update product unit price along with product price + When I update product "product1" prices with following information: + | price | 20 | + | unit_price | 500 | + Then product product1 should have following prices information: + | price | 20 | + | unit_price | 500 | + | unit_price_ratio | 0.04 | + When I update product "product1" prices with following information: + | price | 0 | + | unit_price | 500 | + Then product product1 should have following prices information: + | price | 0 | + | unit_price | 0 | + | unit_price_ratio | 0 | - Scenario: I update product prices providing non-existing tax rules group - Given I add product "product5" with following information: - | name | en-US: black tie | - | is_virtual | false | - And product product5 should have following prices information: - | tax rules group | | - When I update product "product5" prices and apply non-existing tax rules group - Then I should get error that tax rules group does not exist + Scenario: I update product prices providing non-existing tax rules group + Given I update product "product1" prices with following information: + | tax rules group | US-AL Rate (4%) | + And product product1 should have following prices information: + | tax rules group | US-AL Rate (4%) | + When I update product "product1" prices and apply non-existing tax rules group + Then I should get error that tax rules group does not exist + And product product1 should have following prices information: + | tax rules group | US-AL Rate (4%) | From c346249825b815e0b12e93d1e57a766279a3f44d Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 24 Nov 2020 15:45:27 +0200 Subject: [PATCH 16/18] remove unused class from constructor --- .../CommandHandler/UpdateProductPricesHandler.php | 9 --------- .../Resources/config/services/adapter/product.yml | 1 - 2 files changed, 10 deletions(-) diff --git a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php index 2abfd15cca786..b0bbfdce93ae6 100644 --- a/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php +++ b/src/Adapter/Product/CommandHandler/UpdateProductPricesHandler.php @@ -34,7 +34,6 @@ use PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler\UpdateProductPricesHandlerInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\CannotUpdateProductException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; -use PrestaShop\PrestaShop\Core\Util\Number\NumberExtractor; use Product; /** @@ -42,11 +41,6 @@ */ final class UpdateProductPricesHandler implements UpdateProductPricesHandlerInterface { - /** - * @var NumberExtractor - */ - private $numberExtractor; - /** * @var ProductRepository */ @@ -58,16 +52,13 @@ final class UpdateProductPricesHandler implements UpdateProductPricesHandlerInte private $productPricePropertiesFiller; /** - * @param NumberExtractor $numberExtractor * @param ProductRepository $productRepository * @param ProductPricePropertiesFiller $productPricePropertiesFiller */ public function __construct( - NumberExtractor $numberExtractor, ProductRepository $productRepository, ProductPricePropertiesFiller $productPricePropertiesFiller ) { - $this->numberExtractor = $numberExtractor; $this->productRepository = $productRepository; $this->productPricePropertiesFiller = $productPricePropertiesFiller; } diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 1fcad99b2d79d..4fd1e99a24f87 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -102,7 +102,6 @@ services: prestashop.adapter.product.command_handler.update_product_prices_handler: class: PrestaShop\PrestaShop\Adapter\Product\CommandHandler\UpdateProductPricesHandler arguments: - - '@prestashop.core.util.number.number_extractor' - '@prestashop.adapter.product.repository.product_repository' - '@prestashop.adapter.product.product_price_properties_filler' tags: From 6fd939b1ad1279fada6f23d05921cbacea3545e6 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 24 Nov 2020 15:55:48 +0200 Subject: [PATCH 17/18] use number_extractor in Filler --- .../Update/ProductPricePropertiesFiller.php | 19 +++++++++++++++++-- .../config/services/adapter/product.yml | 2 ++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Adapter/Product/Update/ProductPricePropertiesFiller.php b/src/Adapter/Product/Update/ProductPricePropertiesFiller.php index 2688dfe1ae610..ad2205cbf1ed9 100644 --- a/src/Adapter/Product/Update/ProductPricePropertiesFiller.php +++ b/src/Adapter/Product/Update/ProductPricePropertiesFiller.php @@ -29,6 +29,7 @@ namespace PrestaShop\PrestaShop\Adapter\Product\Update; use PrestaShop\Decimal\DecimalNumber; +use PrestaShop\PrestaShop\Core\Util\Number\NumberExtractor; use Product; /** @@ -36,6 +37,20 @@ */ class ProductPricePropertiesFiller { + /** + * @var NumberExtractor + */ + private $numberExtractor; + + /** + * @param NumberExtractor $numberExtractor + */ + public function __construct( + NumberExtractor $numberExtractor + ) { + $this->numberExtractor = $numberExtractor; + } + /** * Wraps following properties filling: price, unit_price, unit_price_ratio, wholesale_price * as most of them (price, unit_price, unit_price_ratio) are highly coupled & depends on each other @@ -58,7 +73,7 @@ public function fillWithPrices(Product $product, ?DecimalNumber $price, ?Decimal $product->price = (float) (string) $price; $updatableProperties[] = 'price'; } else { - $price = new DecimalNumber((string) $product->price); + $price = $this->numberExtractor->extract($product, 'price'); } $this->fillUnitPriceRatio($product, $price, $unitPrice); @@ -82,7 +97,7 @@ private function fillUnitPriceRatio(Product $product, DecimalNumber $price, ?Dec } if (null === $unitPrice) { - $unitPrice = new DecimalNumber((string) $product->unit_price); + $unitPrice = $this->numberExtractor->extract($product, 'unit_price'); } // if price was not reset then allow setting new unit_price and unit_price_ratio diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 4fd1e99a24f87..880e41e5fbf96 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -298,6 +298,8 @@ services: prestashop.adapter.product.product_price_properties_filler: class: PrestaShop\PrestaShop\Adapter\Product\Update\ProductPricePropertiesFiller + arguments: + - '@prestashop.core.util.number.number_extractor' prestashop.adapter.product.validate.customization_field_validator: class: PrestaShop\PrestaShop\Adapter\Product\Validate\CustomizationFieldValidator From 404455350f44a826b9d4ac89092851a92f8ba9e6 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 24 Nov 2020 15:59:18 +0200 Subject: [PATCH 18/18] add code for INVALID_UNITY --- .../Product/Validate/ProductValidator.php | 2 +- .../Exception/ProductConstraintException.php | 65 ++++++++++--------- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/src/Adapter/Product/Validate/ProductValidator.php b/src/Adapter/Product/Validate/ProductValidator.php index 567bdc370ed92..20b24bf239b24 100644 --- a/src/Adapter/Product/Validate/ProductValidator.php +++ b/src/Adapter/Product/Validate/ProductValidator.php @@ -179,7 +179,7 @@ private function validatePrices(Product $product): void } $this->validateProductProperty($product, 'price', ProductConstraintException::INVALID_PRICE); - $this->validateProductProperty($product, 'unity'); + $this->validateProductProperty($product, 'unity', ProductConstraintException::INVALID_UNITY); $this->validateProductProperty($product, 'ecotax', ProductConstraintException::INVALID_ECOTAX); $this->validateProductProperty($product, 'wholesale_price', ProductConstraintException::INVALID_WHOLESALE_PRICE); } diff --git a/src/Core/Domain/Product/Exception/ProductConstraintException.php b/src/Core/Domain/Product/Exception/ProductConstraintException.php index 24ae67dcfe7a9..f258d4f5a3f35 100644 --- a/src/Core/Domain/Product/Exception/ProductConstraintException.php +++ b/src/Core/Domain/Product/Exception/ProductConstraintException.php @@ -66,153 +66,158 @@ class ProductConstraintException extends ProductException */ const INVALID_PRICE = 70; + /** + * When invalid product unity value is supplied + */ + const INVALID_UNITY = 80; + /** * When invalid product ecotax is supplied */ - const INVALID_ECOTAX = 80; + const INVALID_ECOTAX = 90; /** * When invalid product unit price is supplied */ - const INVALID_UNIT_PRICE = 90; + const INVALID_UNIT_PRICE = 100; /** * When invalid product wholesale_price is supplied */ - const INVALID_WHOLESALE_PRICE = 100; + const INVALID_WHOLESALE_PRICE = 110; /** * When product visibility value is invalid */ - const INVALID_VISIBILITY = 110; + const INVALID_VISIBILITY = 120; /** * When product Ean13 code value is invalid */ - const INVALID_EAN_13 = 120; + const INVALID_EAN_13 = 130; /** * When product ISBN code value is invalid */ - const INVALID_ISBN = 130; + const INVALID_ISBN = 140; /** * When product mpn code value is invalid */ - const INVALID_MPN = 140; + const INVALID_MPN = 150; /** * When product upc code value is invalid */ - const INVALID_UPC = 150; + const INVALID_UPC = 160; /** * When product reference value is invalid */ - const INVALID_REFERENCE = 160; + const INVALID_REFERENCE = 170; /** * When product tag value is invalid */ - const INVALID_TAG = 170; + const INVALID_TAG = 180; /** * When product additional time notes type is invalid */ - const INVALID_ADDITIONAL_TIME_NOTES_TYPE = 180; + const INVALID_ADDITIONAL_TIME_NOTES_TYPE = 190; /** * When product width is invalid */ - const INVALID_WIDTH = 190; + const INVALID_WIDTH = 200; /** * When product height is invalid */ - const INVALID_HEIGHT = 200; + const INVALID_HEIGHT = 210; /** * When product depth is invalid */ - const INVALID_DEPTH = 210; + const INVALID_DEPTH = 220; /** * When product weight is invalid */ - const INVALID_WEIGHT = 220; + const INVALID_WEIGHT = 230; /** * When product additional shipping cost is invalid */ - const INVALID_ADDITIONAL_SHIPPING_COST = 230; + const INVALID_ADDITIONAL_SHIPPING_COST = 240; /** * When product delivery time in stock notes are invalid */ - const INVALID_DELIVERY_TIME_IN_STOCK_NOTES = 240; + const INVALID_DELIVERY_TIME_IN_STOCK_NOTES = 250; /** * When product delivery time out of stock notes are invalid */ - const INVALID_DELIVERY_TIME_OUT_OF_STOCK_NOTES = 250; + const INVALID_DELIVERY_TIME_OUT_OF_STOCK_NOTES = 260; /** * When product redirect type is invalid */ - const INVALID_REDIRECT_TYPE = 260; + const INVALID_REDIRECT_TYPE = 270; /** * When product redirect target */ - const INVALID_REDIRECT_TARGET = 270; + const INVALID_REDIRECT_TARGET = 280; /** * When product meta description is invalid */ - const INVALID_META_DESCRIPTION = 280; + const INVALID_META_DESCRIPTION = 290; /** * When product meta title is invalid */ - const INVALID_META_TITLE = 290; + const INVALID_META_TITLE = 300; /** * When product link rewrite is invalid */ - const INVALID_LINK_REWRITE = 300; + const INVALID_LINK_REWRITE = 310; /** * When product minimal quantity is invalid */ - const INVALID_MINIMAL_QUANTITY = 310; + const INVALID_MINIMAL_QUANTITY = 320; /** * When product location is invalid */ - const INVALID_LOCATION = 320; + const INVALID_LOCATION = 330; /** * When product available later labels are invalid */ - const INVALID_AVAILABLE_LATER = 330; + const INVALID_AVAILABLE_LATER = 340; /** * When product available now labels are is invalid */ - const INVALID_AVAILABLE_NOW = 340; + const INVALID_AVAILABLE_NOW = 350; /** * When product available date is invalid */ - const INVALID_AVAILABLE_DATE = 350; + const INVALID_AVAILABLE_DATE = 360; /** * When product low stock alert is invalid */ - const INVALID_LOW_STOCK_ALERT = 360; + const INVALID_LOW_STOCK_ALERT = 370; /** * When product low stock threshold is invalid */ - const INVALID_LOW_STOCK_THRESHOLD = 370; + const INVALID_LOW_STOCK_THRESHOLD = 380; }