From a819a2fada8fdb2c081a9c81d3a728e8166642c5 Mon Sep 17 00:00:00 2001 From: Michael Telgmann Date: Thu, 21 Sep 2023 13:09:30 +0200 Subject: [PATCH 1/2] test: refactor existing savePositionAction test a bit --- .../Controllers/Backend/OrderTest.php | 66 +++++++++++-------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/tests/Functional/Controllers/Backend/OrderTest.php b/tests/Functional/Controllers/Backend/OrderTest.php index f458af18d0a..4e437173b86 100644 --- a/tests/Functional/Controllers/Backend/OrderTest.php +++ b/tests/Functional/Controllers/Backend/OrderTest.php @@ -67,11 +67,16 @@ class OrderTest extends ControllerTestCase private const GERMANY_COUNTRY_ID = 2; private const NRW_STATE_ID = 3; private const GERMANY_AREA_ID = 1; - private const PRODUCT_GRADUATED_PRICES_DEMODATA_ORDERNUMBER = 'SW10208'; + private const PRODUCT_GRADUATED_PRICES_DEMODATA_ORDER_NUMBER = 'SW10208'; + private const PRODUCT_GRADUATED_PRICES_DEMODATA_PRODUCT_ID = 209; + private const PRODUCT_GRADUATED_PRICES_DEMODATA_PRODUCT_VARIANT_ID = 747; private const PRODUCT_GRADUATED_PRICES_DEMODATA_NAME = 'Staffelpreise'; - private const PRODUCT_GRADUATED_PRICES_VARIANT1_ORDERNUMBER = 'SW10090.1'; - private const PRODUCT_GRADUATED_PRICES_VARIANT2_ORDERNUMBER = 'SW10090.2'; + private const PRODUCT_GRADUATED_PRICES_VARIANT1_ORDER_NUMBER = 'SW10090.1'; + private const PRODUCT_GRADUATED_PRICES_VARIANT1_PRODUCT_VARIANT_ID = 153; + private const PRODUCT_GRADUATED_PRICES_VARIANT2_ORDER_NUMBER = 'SW10090.2'; + private const PRODUCT_GRADUATED_PRICES_VARIANT2_PRODUCT_VARIANT_ID = 154; private const PRODUCT_GRADUATED_PRICES_VARIANT_NAME = 'Teigschaber'; + private const PRODUCT_GRADUATED_PRICES_VARIANT_PRODUCT_ID = 89; private Connection $connection; @@ -242,25 +247,18 @@ public function testSavingOrderWithDifferentTimeZone(): void * * @throws Exception */ - public function testSavePositionActionReturnValuesForGraduatedPrices(array $params, array $expectedValues): void + public function testSavePositionActionReturnValuesForGraduatedPricesByAddingNewPositions(array $params, array $expectedValues): void { - $order = $this->modelManager->find(Order::class, $params['orderId']); - static::assertInstanceOf(Order::class, $order); - $productPricesSql = file_get_contents(__DIR__ . '/_fixtures/article/graduatedPrices.sql'); static::assertIsString($productPricesSql); $this->connection->executeQuery($productPricesSql); + $order = $this->modelManager->find(Order::class, $params['orderId']); + static::assertInstanceOf(Order::class, $order); $request = new Enlight_Controller_Request_RequestTestCase(); - $request->setParams([ + $request->setParams(array_merge([ 'id' => 0, - 'orderId' => $params['orderId'], 'mode' => 0, - 'articleId' => 9, - 'articleDetailId' => null, - 'articleNumber' => $params['productNumber'], - 'articleName' => $params['productName'], - 'quantity' => $params['quantity'], 'statusId' => 0, 'statusDescription' => '', 'taxId' => 1, @@ -268,7 +266,7 @@ public function testSavePositionActionReturnValuesForGraduatedPrices(array $para 'taxDescription' => '', 'inStock' => 0, 'changed' => $order->getChanged() ? $order->getChanged()->format(DateTimeInterface::ATOM) : '', - ]); + ], $params)); $controller = $this->getController(); $controller->setRequest($request); @@ -281,15 +279,17 @@ public function testSavePositionActionReturnValuesForGraduatedPrices(array $para } /** - * @return Generator + * @return Generator */ public function provideProductParamsForSavePositionActionTestingGraduatedPrices(): Generator { yield 'customer-group H with netto-shop-price-config has to return fallback prices for EK' => [ 'params' => [ 'orderId' => self::ORDER_ID_DEMODATA_H, - 'productNumber' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_ORDERNUMBER, - 'productName' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_NAME, + 'articleNumber' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_ORDER_NUMBER, + 'articleName' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_NAME, + 'articleId' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_PRODUCT_ID, + 'articleDetailId' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_PRODUCT_VARIANT_ID, 'quantity' => 30, ], 'expectedValues' => [ @@ -300,8 +300,10 @@ public function provideProductParamsForSavePositionActionTestingGraduatedPrices( yield 'customer-group EK' => [ 'params' => [ 'orderId' => self::ORDER_ID_DEMODATA_EK, - 'productNumber' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_ORDERNUMBER, - 'productName' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_NAME, + 'articleNumber' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_ORDER_NUMBER, + 'articleName' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_NAME, + 'articleId' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_PRODUCT_ID, + 'articleDetailId' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_PRODUCT_VARIANT_ID, 'quantity' => 30, ], 'expectedValues' => [ @@ -312,8 +314,10 @@ public function provideProductParamsForSavePositionActionTestingGraduatedPrices( yield 'product with variants - variant 1 - customer-group EK' => [ 'params' => [ 'orderId' => self::ORDER_ID_DEMODATA_EK, - 'productNumber' => self::PRODUCT_GRADUATED_PRICES_VARIANT1_ORDERNUMBER, - 'productName' => self::PRODUCT_GRADUATED_PRICES_VARIANT_NAME, + 'articleNumber' => self::PRODUCT_GRADUATED_PRICES_VARIANT1_ORDER_NUMBER, + 'articleName' => self::PRODUCT_GRADUATED_PRICES_VARIANT_NAME, + 'articleId' => self::PRODUCT_GRADUATED_PRICES_VARIANT_PRODUCT_ID, + 'articleDetailId' => self::PRODUCT_GRADUATED_PRICES_VARIANT1_PRODUCT_VARIANT_ID, 'quantity' => 5, ], 'expectedValues' => [ @@ -324,8 +328,10 @@ public function provideProductParamsForSavePositionActionTestingGraduatedPrices( yield 'product with variants - variant 2 - customer-group EK' => [ 'params' => [ 'orderId' => self::ORDER_ID_DEMODATA_EK, - 'productNumber' => self::PRODUCT_GRADUATED_PRICES_VARIANT2_ORDERNUMBER, - 'productName' => self::PRODUCT_GRADUATED_PRICES_VARIANT_NAME, + 'articleNumber' => self::PRODUCT_GRADUATED_PRICES_VARIANT2_ORDER_NUMBER, + 'articleName' => self::PRODUCT_GRADUATED_PRICES_VARIANT_NAME, + 'articleId' => self::PRODUCT_GRADUATED_PRICES_VARIANT_PRODUCT_ID, + 'articleDetailId' => self::PRODUCT_GRADUATED_PRICES_VARIANT2_PRODUCT_VARIANT_ID, 'quantity' => 21, ], 'expectedValues' => [ @@ -336,8 +342,10 @@ public function provideProductParamsForSavePositionActionTestingGraduatedPrices( yield 'product with variants - variant 1 - customer-group H - netto' => [ 'params' => [ 'orderId' => self::ORDER_ID_DEMODATA_H, - 'productNumber' => self::PRODUCT_GRADUATED_PRICES_VARIANT1_ORDERNUMBER, - 'productName' => self::PRODUCT_GRADUATED_PRICES_VARIANT_NAME, + 'articleNumber' => self::PRODUCT_GRADUATED_PRICES_VARIANT1_ORDER_NUMBER, + 'articleName' => self::PRODUCT_GRADUATED_PRICES_VARIANT_NAME, + 'articleId' => self::PRODUCT_GRADUATED_PRICES_VARIANT_PRODUCT_ID, + 'articleDetailId' => self::PRODUCT_GRADUATED_PRICES_VARIANT1_PRODUCT_VARIANT_ID, 'quantity' => 10, ], 'expectedValues' => [ @@ -348,8 +356,10 @@ public function provideProductParamsForSavePositionActionTestingGraduatedPrices( yield 'product with variants - variant 2 - customer-group H - netto' => [ 'params' => [ 'orderId' => self::ORDER_ID_DEMODATA_H, - 'productNumber' => self::PRODUCT_GRADUATED_PRICES_VARIANT2_ORDERNUMBER, - 'productName' => self::PRODUCT_GRADUATED_PRICES_VARIANT_NAME, + 'articleNumber' => self::PRODUCT_GRADUATED_PRICES_VARIANT2_ORDER_NUMBER, + 'articleName' => self::PRODUCT_GRADUATED_PRICES_VARIANT_NAME, + 'articleId' => self::PRODUCT_GRADUATED_PRICES_VARIANT_PRODUCT_ID, + 'articleDetailId' => self::PRODUCT_GRADUATED_PRICES_VARIANT2_PRODUCT_VARIANT_ID, 'quantity' => 42, ], 'expectedValues' => [ From c491cd43fa7b6c3e73eb3a39dcb12b8b6f2a47e0 Mon Sep 17 00:00:00 2001 From: Michael Telgmann Date: Thu, 21 Sep 2023 14:47:46 +0200 Subject: [PATCH 2/2] fix(SW-27165): Change consideration of graduated prices in order module While adding new positions to an existing order in the backend order module, graduated prices are still considered. If an existing position is changed, the graduated price will only be checked and considered if the quantity was changed. With this change it is possible again to also set a custom price for a position that contains a product with graduated prices. --- engine/Shopware/Controllers/Backend/Order.php | 31 +++--- .../Controllers/Backend/OrderTest.php | 98 +++++++++++++++++++ 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/engine/Shopware/Controllers/Backend/Order.php b/engine/Shopware/Controllers/Backend/Order.php index e24e110db9b..71f9b742001 100644 --- a/engine/Shopware/Controllers/Backend/Order.php +++ b/engine/Shopware/Controllers/Backend/Order.php @@ -117,7 +117,7 @@ class Shopware_Controllers_Backend_Order extends Shopware_Controllers_Backend_Ex /** * @deprecated - Will be removed in Shopware 5.8 without a replacement - * Contains the dynamic receipt repository + * Contains the order document repository * * @var OrderDocumentRepository */ @@ -1950,7 +1950,17 @@ private function getPositionAssociatedData(array $data, Order $order): array $shopContext = $this->createShopContext($order); $data = $this->checkTaxRule($data, $shopContext); - if ($this->hasProductGraduatedPrices($data['articleNumber'], $order)) { + $orderPosition = null; + foreach ($order->getDetails() as $position) { + if ($position->getId() === (int) $data['id']) { + $orderPosition = $position; + break; + } + } + // Only get graduated price if a new position should be added or quantity has changed + if (($orderPosition === null || $orderPosition->getQuantity() !== (int) $data['quantity']) + && $this->hasProductGraduatedPrices($data['articleNumber'], $order) + ) { $data = $this->checkPrice($data, $order, $shopContext); } @@ -2106,20 +2116,15 @@ private function checkTaxRule(array $data, ShopContextInterface $shopContext): a return $data; } - private function hasProductGraduatedPrices(string $productNumber, ORDER $order): bool + private function hasProductGraduatedPrices(string $productNumber, Order $order): bool { $customerGroupKey = $this->getCustomerGroupKey($order); - $sql = 'SELECT - prices.pricegroup, count(*) - FROM - s_articles_prices AS prices - INNER JOIN - s_articles_details ON prices.articledetailsID = s_articles_details.id - WHERE - s_articles_details.ordernumber = :productNumber - GROUP BY - prices.pricegroup;'; + $sql = 'SELECT prices.pricegroup, count(*) + FROM s_articles_prices AS prices + INNER JOIN s_articles_details ON prices.articledetailsID = s_articles_details.id + WHERE s_articles_details.ordernumber = :productNumber + GROUP BY prices.pricegroup;'; $result = $this->container->get(Connection::class)->executeQuery($sql, ['productNumber' => $productNumber])->fetchAllKeyValue(); diff --git a/tests/Functional/Controllers/Backend/OrderTest.php b/tests/Functional/Controllers/Backend/OrderTest.php index 4e437173b86..9c6ed5c9935 100644 --- a/tests/Functional/Controllers/Backend/OrderTest.php +++ b/tests/Functional/Controllers/Backend/OrderTest.php @@ -369,6 +369,104 @@ public function provideProductParamsForSavePositionActionTestingGraduatedPrices( ]; } + /** + * @dataProvider provideProductParamsForSavePositionActionOnExistingPosition + * + * @param array{quantity?: int, price?: float, total?: float} $params + * @param array{price: float, total: float} $expectedValues + */ + public function testSavePositionActionReturnValuesForGraduatedPricesOnExistingPosition(array $params, array $expectedValues): void + { + $order = $this->modelManager->getRepository(Order::class)->findOneBy([]); + static::assertInstanceOf(Order::class, $order); + $newPositionParams = [ + 'id' => 0, + 'orderId' => $order->getId(), + 'articleNumber' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_ORDER_NUMBER, + 'articleName' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_NAME, + 'articleId' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_PRODUCT_ID, + 'articleDetailId' => self::PRODUCT_GRADUATED_PRICES_DEMODATA_PRODUCT_VARIANT_ID, + 'mode' => 0, + 'quantity' => 2, + 'statusId' => 0, + 'statusDescription' => '', + 'taxId' => 1, + 'taxRate' => 19.0, + 'taxDescription' => '', + 'inStock' => 0, + 'changed' => $order->getChanged() ? $order->getChanged()->format(DateTimeInterface::ATOM) : '', + ]; + + $request = new Enlight_Controller_Request_RequestTestCase(); + $request->setParams($newPositionParams); + + $controller = $this->getController(); + $controller->setRequest($request); + $controller->savePositionAction(); + $results = $controller->View()->getAssign(); + + static::assertTrue($results['success'], $results['message'] ?? ''); + static::assertSame(0.84, $results['data']['price']); + static::assertSame(1.68, $results['data']['total']); + + $this->modelManager->refresh($order); + $changeQuantityParams = [ + 'id' => $results['data']['id'], + 'price' => $results['data']['price'], + 'total' => $results['data']['total'], + 'changed' => $order->getChanged() ? $order->getChanged()->format(DateTimeInterface::ATOM) : '', + ]; + $changeQuantityParams = array_merge($newPositionParams, $changeQuantityParams, $params); + + $request = new Enlight_Controller_Request_RequestTestCase(); + $request->setParams($changeQuantityParams); + + $controller->setRequest($request); + $controller->savePositionAction(); + $results = $controller->View()->getAssign(); + + static::assertTrue($results['success'], $results['message'] ?? ''); + static::assertSame($expectedValues['price'], $results['data']['price']); + static::assertSame($expectedValues['total'], $results['data']['total']); + } + + /** + * @return Generator + */ + public function provideProductParamsForSavePositionActionOnExistingPosition(): Generator + { + yield 'Only change quantity, graduated price should be considered' => [ + 'params' => [ + 'quantity' => 20, + ], + 'expectedValues' => [ + 'price' => 0.76, + 'total' => 15.2, + ], + ]; + yield 'Only change price, graduated price should be ignored' => [ + 'params' => [ + 'price' => 0.15, + 'total' => 0.30, + ], + 'expectedValues' => [ + 'price' => 0.15, + 'total' => 0.30, + ], + ]; + yield 'Change quantity and price, graduated price should be considered' => [ + 'params' => [ + 'quantity' => 30, + 'price' => 0.15, + 'total' => 0.30, + ], + 'expectedValues' => [ + 'price' => 0.67, + 'total' => 20.1, + ], + ]; + } + /** * @dataProvider provideTaxRuleParams */