From 5e078be95640c2b2efb5c7e8056a3b95db8d37b8 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Fri, 11 Dec 2020 21:40:53 +0200 Subject: [PATCH 1/6] behat for wholesale_price change when assigning supplier --- .../UpdateProductSuppliersFeatureContext.php | 4 +- .../Scenario/Product/update_suppliers.feature | 64 ++++++++++++++++++- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/UpdateProductSuppliersFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/UpdateProductSuppliersFeatureContext.php index 013a2b31347ea..21dded3348507 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/UpdateProductSuppliersFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/UpdateProductSuppliersFeatureContext.php @@ -44,11 +44,11 @@ class UpdateProductSuppliersFeatureContext extends AbstractProductFeatureContext { /** - * @When I delete all product :productReference suppliers + * @When I remove all associated product :productReference suppliers * * @param string $productReference */ - public function deleteAllProductSuppliers(string $productReference): void + public function removeAssociatedProductSuppliers(string $productReference): void { try { $this->getCommandBus()->handle(new RemoveAllAssociatedProductSuppliersCommand( diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/update_suppliers.feature b/tests/Integration/Behaviour/Features/Scenario/Product/update_suppliers.feature index 4a3b46bdfa5f1..fc8a683458259 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/update_suppliers.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/update_suppliers.feature @@ -1,5 +1,6 @@ # ./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s product --tags update-suppliers @reset-database-before-feature +@clear-cache-before-feature @update-suppliers Feature: Update product suppliers from Back Office (BO) As a BO user @@ -60,7 +61,7 @@ Feature: Update product suppliers from Back Office (BO) | default supplier | supplier2 | | default supplier reference | my second supplier for product1 | - Scenario: Remove product suppliers + Scenario: Remove all associated product suppliers Given product product1 type should be standard And product product1 should have following suppliers: | product supplier reference | currency | price tax excluded | @@ -69,7 +70,7 @@ Feature: Update product suppliers from Back Office (BO) And product product1 should have following supplier values: | default supplier | supplier2 | | default supplier reference | my second supplier for product1 | - When I delete all product product1 suppliers + When I remove all associated product product1 suppliers Then product product1 should not have any suppliers assigned And product product1 should not have a default supplier And product product1 default supplier reference should be empty @@ -105,3 +106,62 @@ Feature: Update product suppliers from Back Office (BO) | sup white shirt M 1 | USD | 5 | whiteM | | sup white shirt L 2 | USD | 3 | whiteL | Then product product2 default supplier reference should be empty + + Scenario: Standard product wholesale price should depend on default supplier price + Given I add product "product3" with following information: + | name[en-US] | magic staff | + | is_virtual | false | + And product product3 type should be standard + And product product3 should not have any suppliers assigned + And product product3 should have following prices information: + | wholesale_price | 0 | + When I set product product3 default supplier to supplier1 and following suppliers: + | reference | supplier reference | product supplier reference | currency | price tax excluded | + | product3supplier1 | supplier1 | my first supplier for product3 | USD | 10 | + Then product product3 should have following suppliers: + | product supplier reference | currency | price tax excluded | + | my first supplier for product3 | USD | 10 | + And product product3 should have following supplier values: + | default supplier | supplier1 | + And product product3 should have following prices information: + | wholesale_price | 10 | + When I remove all associated product "product3" suppliers + Then product product3 should not have any suppliers assigned + And product product3 should not have a default supplier + And product product3 default supplier reference should be empty + And product product3 should have following prices information: + | wholesale_price | 0 | + + Scenario: Combination product wholesale price should be reset when default supplier is assigned + Given I add product "product4" with following information: + | name[en-US] | regular T-shirt | + | is_virtual | false | + And I update product product4 prices with following information: + | wholesale_price | 15 | + And product "product4" has following combinations: + | reference | quantity | attributes | + | whiteM | 15 | Size:M;Color:White | + | whiteL | 13 | Size:L;Color:White | + And product product4 type should be combination + And product product4 should not have any suppliers assigned + And product product4 default supplier reference should be empty + And product product4 should have following prices information: + | wholesale_price | 15 | + When I set product product4 default supplier to supplier1 and following suppliers: + | reference | supplier reference | product supplier reference | currency | price tax excluded | combination | + | product4whiteM | supplier1 | sup white shirt M 1 | USD | 5 | whiteM | + | product4whiteL | supplier1 | sup white shirt L 2 | USD | 3 | whiteL | + Then product product4 should have following suppliers: + | product supplier reference | currency | price tax excluded | combination | + | sup white shirt M 1 | USD | 5 | whiteM | + | sup white shirt L 2 | USD | 3 | whiteL | + Then product product4 default supplier reference should be empty + And product product4 should have following prices information: + | wholesale_price | 0 | + And I update product product4 prices with following information: + | wholesale_price | 11 | + When I remove all associated product product4 suppliers + And product product4 should not have any suppliers assigned + And product product4 default supplier reference should be empty + And product product4 should have following prices information: + | wholesale_price | 0 | From 1e94d9903857ed0defb0cb5eff8b9ecb24571b3a Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 15 Dec 2020 11:56:03 +0200 Subject: [PATCH 2/6] fix caching issue related to ps_facetedsearch --- .../Product/Update/ProductSupplierUpdater.php | 3 + .../Scenario/Product/update_suppliers.feature | 74 +++++++++++++++++-- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/Adapter/Product/Update/ProductSupplierUpdater.php b/src/Adapter/Product/Update/ProductSupplierUpdater.php index 288589f2872cf..39c4da5030b98 100644 --- a/src/Adapter/Product/Update/ProductSupplierUpdater.php +++ b/src/Adapter/Product/Update/ProductSupplierUpdater.php @@ -109,6 +109,9 @@ public function resetDefaultSupplier(Product $product): void $product->wholesale_price = '0'; $product->id_supplier = 0; + // Important for ps_facetedsearch module, without this it raises caching issues by invoking Product::priceCalculation() + Product::flushPriceCache(); + $this->productRepository->partialUpdate( $product, ['supplier_reference', 'wholesale_price', 'id_supplier'], diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/update_suppliers.feature b/tests/Integration/Behaviour/Features/Scenario/Product/update_suppliers.feature index fc8a683458259..82b3c49aa0031 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/update_suppliers.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/update_suppliers.feature @@ -114,7 +114,14 @@ Feature: Update product suppliers from Back Office (BO) And product product3 type should be standard And product product3 should not have any suppliers assigned And product product3 should have following prices information: - | wholesale_price | 0 | + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 0 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | When I set product product3 default supplier to supplier1 and following suppliers: | reference | supplier reference | product supplier reference | currency | price tax excluded | | product3supplier1 | supplier1 | my first supplier for product3 | USD | 10 | @@ -124,18 +131,41 @@ Feature: Update product suppliers from Back Office (BO) And product product3 should have following supplier values: | default supplier | supplier1 | And product product3 should have following prices information: - | wholesale_price | 10 | + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 10 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | When I remove all associated product "product3" suppliers Then product product3 should not have any suppliers assigned And product product3 should not have a default supplier And product product3 default supplier reference should be empty And product product3 should have following prices information: - | wholesale_price | 0 | + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 0 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | Scenario: Combination product wholesale price should be reset when default supplier is assigned Given I add product "product4" with following information: | name[en-US] | regular T-shirt | | is_virtual | false | + And product product4 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 | And I update product product4 prices with following information: | wholesale_price | 15 | And product "product4" has following combinations: @@ -146,7 +176,14 @@ Feature: Update product suppliers from Back Office (BO) And product product4 should not have any suppliers assigned And product product4 default supplier reference should be empty And product product4 should have following prices information: - | wholesale_price | 15 | + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 15 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | When I set product product4 default supplier to supplier1 and following suppliers: | reference | supplier reference | product supplier reference | currency | price tax excluded | combination | | product4whiteM | supplier1 | sup white shirt M 1 | USD | 5 | whiteM | @@ -157,11 +194,34 @@ Feature: Update product suppliers from Back Office (BO) | sup white shirt L 2 | USD | 3 | whiteL | Then product product4 default supplier reference should be empty And product product4 should have following prices information: - | wholesale_price | 0 | + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 0 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | And I update product product4 prices with following information: | wholesale_price | 11 | + And product product4 should have following prices information: + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 11 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | When I remove all associated product product4 suppliers - And product product4 should not have any suppliers assigned + Then product product4 should not have any suppliers assigned And product product4 default supplier reference should be empty And product product4 should have following prices information: - | wholesale_price | 0 | + | price | 0 | + | ecotax | 0 | + | tax rules group | | + | on_sale | false | + | wholesale_price | 0 | + | unit_price | 0 | + | unity | | + | unit_price_ratio | 0 | From 37b4df2b2cea188a4a945a27abf78eb3c13cea76 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 15 Dec 2020 12:14:36 +0200 Subject: [PATCH 3/6] dont group by suppliers when getting for removal --- .../RemoveAllAssociatedProductSuppliersHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Product/CommandHandler/RemoveAllAssociatedProductSuppliersHandler.php b/src/Adapter/Product/CommandHandler/RemoveAllAssociatedProductSuppliersHandler.php index 3c5f45763431f..506064e2c2bcd 100644 --- a/src/Adapter/Product/CommandHandler/RemoveAllAssociatedProductSuppliersHandler.php +++ b/src/Adapter/Product/CommandHandler/RemoveAllAssociatedProductSuppliersHandler.php @@ -79,7 +79,7 @@ public function handle(RemoveAllAssociatedProductSuppliersCommand $command): voi $product = $this->productRepository->get($command->getProductId()); $productSupplierIds = []; - foreach (ProductSupplier::getSupplierCollection($product->id) as $productSupplier) { + foreach (ProductSupplier::getSupplierCollection($product->id, false) as $productSupplier) { $productSupplierIds[] = new ProductSupplierId((int) $productSupplier->id); } From c00e3ef05431e7d759943139df37718c3f9751bb Mon Sep 17 00:00:00 2001 From: Jonathan Lelievre Date: Tue, 15 Dec 2020 12:32:19 +0100 Subject: [PATCH 4/6] Improve price cache checking in case combination did not exist the first time --- classes/Product.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/classes/Product.php b/classes/Product.php index f07b53af2d132..e71be120ac3de 100644 --- a/classes/Product.php +++ b/classes/Product.php @@ -3700,7 +3700,9 @@ public static function priceCalculation( // fetch price & attribute price $cache_id_2 = $id_product . '-' . $id_shop; - if (!isset(self::$_pricesLevel2[$cache_id_2])) { + // We need to check the cache for this price AND attribute, if absent the whole product cache needs update + // This can happen if the cache was filled before the combination was created for example + if (!isset(self::$_pricesLevel2[$cache_id_2][(int) $id_product_attribute])) { $sql = new DbQuery(); $sql->select('product_shop.`price`, product_shop.`ecotax`'); $sql->from('product', 'p'); From c9b30c70ca2fff74adc7889fe0e88daf1064a958 Mon Sep 17 00:00:00 2001 From: Jonathan Lelievre Date: Tue, 15 Dec 2020 12:32:36 +0100 Subject: [PATCH 5/6] Add module to gitignore list --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 98301f36e3c17..f6e40965d72c5 100644 --- a/.gitignore +++ b/.gitignore @@ -103,6 +103,9 @@ tests-legacy/resources/modules/ps_categorytree tests-legacy/resources/modules/ps_facetedsearch tests-legacy/resources/modules/ps_sharebuttons +# Installed with test data maybe via API push +tests-legacy/resources/modules/ps_reminder + tests/Resources/img/l/* !tests/Resources/img/l/.gitkeep tests/Resources/img/p/* From 91dac20d066a3d3329ea1999397d3e71a25d2f6d Mon Sep 17 00:00:00 2001 From: Jonathan Lelievre Date: Tue, 15 Dec 2020 12:33:42 +0100 Subject: [PATCH 6/6] Clear product cache in test context rather than in Product updater service --- src/Adapter/Product/Update/ProductSupplierUpdater.php | 3 --- .../Context/Domain/Product/CommonProductFeatureContext.php | 6 ++++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Adapter/Product/Update/ProductSupplierUpdater.php b/src/Adapter/Product/Update/ProductSupplierUpdater.php index 39c4da5030b98..288589f2872cf 100644 --- a/src/Adapter/Product/Update/ProductSupplierUpdater.php +++ b/src/Adapter/Product/Update/ProductSupplierUpdater.php @@ -109,9 +109,6 @@ public function resetDefaultSupplier(Product $product): void $product->wholesale_price = '0'; $product->id_supplier = 0; - // Important for ps_facetedsearch module, without this it raises caching issues by invoking Product::priceCalculation() - Product::flushPriceCache(); - $this->productRepository->partialUpdate( $product, ['supplier_reference', 'wholesale_price', 'id_supplier'], diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php index 4f9f8c69f750f..075cdb396582e 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/CommonProductFeatureContext.php @@ -33,6 +33,7 @@ use PHPUnit\Framework\Assert; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductConstraintException; use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductNotFoundException; +use Product; use RuntimeException; use Tests\Integration\Behaviour\Features\Context\Util\CombinationDetails; use Tests\Integration\Behaviour\Features\Context\Util\ProductCombinationFactory; @@ -67,6 +68,11 @@ public function addCombinationsToProduct(string $productReference, TableNode $ta foreach ($combinations as $combination) { $this->getSharedStorage()->set($combination->reference, (int) $combination->id); } + + // Product class has a lot of cache that is set as soon as the product is created, including for prices + // which are cached for each combinations. Since it was cached when the combinations did not exist we need + // to clear it so that the newly created combinations' prices are correctly computed next time they are needed. + Product::resetStaticCache(); } /**