From 2863527a9dc012b92dceb8fe6f067db59c5f4879 Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Tue, 15 Mar 2022 10:52:42 +0100 Subject: [PATCH 01/11] [Core][Shipping][Behat] Add estimated shipping cost scenarios --- ...shipping_cost_based_on_order_total.feature | 25 +++++++++++++++++ ...hipping_cost_based_on_total_weight.feature | 27 +++++++++++++++++++ .../Behat/Context/Api/Shop/CartContext.php | 9 ++++--- 3 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature create mode 100644 features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_total_weight.feature diff --git a/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature new file mode 100644 index 0000000000..aa102f8334 --- /dev/null +++ b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature @@ -0,0 +1,25 @@ +@applying_shipping_method_rules +Feature: Seeing estimated shipping costs based on order total + In order to be aware of estimated shipping costs + As a Customer + I want to see estimated shipping costs that match the shipping method rule + + Background: + Given the store operates on a single channel in "United States" + And the store has a product "Cheap Jacket" priced at "$20.00" + And the store has a product "Expensive Jacket" priced at "$200.00" + And the store has "Ship with us, ship with pride" shipping method with "$200" fee + And this shipping method is only available for orders over or equal to "$30" + And the store has "We delivery cheap goodz" shipping method with "$2" fee + And this shipping method is only available for orders under or equal to "$29.99" + And I am a logged in customer + + @ui @api + Scenario: Seeing estimated shipping cost that handle expensive goods + When I add product "Expensive Jacket" to the cart + Then my cart estimated shipping cost should be "$200.00" + + @ui @api + Scenario: Seeing estimated shipping cost that handle cheap goods + Given I add product "Cheap Jacket" to the cart + Then my cart estimated shipping cost should be "$2.00" diff --git a/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_total_weight.feature b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_total_weight.feature new file mode 100644 index 0000000000..2fc88a41e9 --- /dev/null +++ b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_total_weight.feature @@ -0,0 +1,27 @@ +@applying_shipping_method_rules +Feature: Seeing estimated shipping costs based on total weight + In order to be aware of estimated shipping costs + As a Customer + I want to see estimated shipping costs that match the shipping method rule + + Background: + Given the store operates on a single channel in "United States" + And the store has a product "Jacket for the Lochness Monster" priced at "$1337.00" + And this product's weight is 200 + And the store has a product "T-Shirt for Tinkerbell" priced at "$1.00" + And this product's weight is 0.1 + And the store has "Heavy Duty Courier" shipping method with "$200" fee + And this shipping method is only available for orders with a total weight greater or equal to 100.0 + And the store has "Fairytale Delivery Service" shipping method with "$2" fee + And this shipping method is only available for orders with a total weight less or equal to 1.0 + And I am a logged in customer + + @ui + Scenario: Seeing estimated shipping cost that handle heavy goods + When I add product "Jacket for the Lochness Monster" to the cart + Then my cart estimated shipping cost should be "$200.00" + + @ui + Scenario: Seeing estimated shipping cost that handle cheap goods + When I add product "T-Shirt for Tinkerbell" to the cart + Then my cart estimated shipping cost should be "$2.00" diff --git a/src/Sylius/Behat/Context/Api/Shop/CartContext.php b/src/Sylius/Behat/Context/Api/Shop/CartContext.php index 53f88263c8..58ad0b3318 100644 --- a/src/Sylius/Behat/Context/Api/Shop/CartContext.php +++ b/src/Sylius/Behat/Context/Api/Shop/CartContext.php @@ -270,9 +270,9 @@ final class CartContext implements Context { Assert::same( $this->responseChecker->getValue( - $this->cartsClient->getLastResponse(), - 'localeCode' - ), + $this->cartsClient->getLastResponse(), + 'localeCode' + ), $locale->getCode() ); } @@ -302,7 +302,7 @@ final class CartContext implements Context * @Then /^my (cart)'s total should be ("[^"]+")$/ * @Then /^my (cart) total should be ("[^"]+")$/ */ - public function myCartSTotalShouldBe(string $tokenValue, int $total): void + public function myCartTotalShouldBe(string $tokenValue, int $total): void { $responseTotal = $this->responseChecker->getValue( $this->cartsClient->show($tokenValue), @@ -547,6 +547,7 @@ final class CartContext implements Context /** * @Then /^my cart shipping total should be ("[^"]+")$/ * @Then I should not see shipping total for my cart + * @Then /^my cart estimated shipping cost should be ("[^"]+")/ */ public function myCartShippingFeeShouldBe(int $shippingTotal = 0): void { From caa04093d1ec25bce846395f0472dc5cbd1dcfab Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Tue, 15 Mar 2022 10:53:52 +0100 Subject: [PATCH 02/11] [Core][Shipping] Change order processing priorities --- ...stimated_shipping_cost_based_on_order_total.feature | 2 +- .../Resources/config/services/order_processing.xml | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature index aa102f8334..edff32dfd5 100644 --- a/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature +++ b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature @@ -21,5 +21,5 @@ Feature: Seeing estimated shipping costs based on order total @ui @api Scenario: Seeing estimated shipping cost that handle cheap goods - Given I add product "Cheap Jacket" to the cart + When I add product "Cheap Jacket" to the cart Then my cart estimated shipping cost should be "$2.00" diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services/order_processing.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services/order_processing.xml index 5d50634256..e206fccf45 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/order_processing.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/order_processing.xml @@ -27,15 +27,15 @@ + + + + + - - - - - From d6dbbcdd785236c85a00416ecca5f19b9b68066a Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Tue, 15 Mar 2022 12:23:28 +0100 Subject: [PATCH 03/11] [Core][Shipping] Update UPGRADE-1.10.md --- UPGRADE-1.10.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/UPGRADE-1.10.md b/UPGRADE-1.10.md index 4e19778c8e..829ca4a69a 100644 --- a/UPGRADE-1.10.md +++ b/UPGRADE-1.10.md @@ -1,3 +1,24 @@ +# UPGRADE FROM `v1.10.x` TO `v1.10.12` +1. Order Processors' priorities have changed and `sylius.order_processing.order_prices_recalculator` has now a higher priority than `sylius.order_processing.order_shipment_processor`. + +Previous priorities: +```shell +sylius.order_processing.order_adjustments_clearer 60 Sylius\Component\Core\OrderProcessing\OrderAdjustmentsClearer +sylius.order_processing.order_shipment_processor 50 Sylius\Component\Core\OrderProcessing\OrderShipmentProcessor +sylius.order_processing.order_prices_recalculator 40 Sylius\Component\Core\OrderProcessing\OrderPricesRecalculator +... +``` + +Current priorities: +```shell +sylius.order_processing.order_adjustments_clearer 60 Sylius\Component\Core\OrderProcessing\OrderAdjustmentsClearer +sylius.order_processing.order_prices_recalculator 50 Sylius\Component\Core\OrderProcessing\OrderPricesRecalculator +sylius.order_processing.order_shipment_processor 40 Sylius\Component\Core\OrderProcessing\OrderShipmentProcessor +... +``` + +If you rely on previous priorities, you can bring them back by setting flag ``shipment_processor_before_prices_recalculator`` to ``true`` in ``config/packages/_sylius.yaml``. However, it is not recommended because new priorities fix [invalid estimated shipping costs](https://github.com/Sylius/Sylius/pull/13769). + # UPGRADE FROM `v1.10.8` TO `v1.10.10` 1. Field `createdByGuest` has been added to `Sylius\Component\Core\Model\Order`, this change will allow us to distinguish carts From 9d9f8d9b18573d361672b77dc2a93b08728be85b Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Tue, 15 Mar 2022 13:31:39 +0100 Subject: [PATCH 04/11] [Core][Shipping][Behat] Update estimated shipping cost scenarios --- ...g_estimated_shipping_cost_based_on_order_total.feature | 4 ++-- ..._estimated_shipping_cost_based_on_total_weight.feature | 8 ++++---- src/Sylius/Behat/Context/Api/Shop/CartContext.php | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature index edff32dfd5..24c9b4320a 100644 --- a/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature +++ b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_order_total.feature @@ -15,11 +15,11 @@ Feature: Seeing estimated shipping costs based on order total And I am a logged in customer @ui @api - Scenario: Seeing estimated shipping cost that handle expensive goods + Scenario: Seeing valid estimated shipping cost for the cart with a value over minimum price configured on the shipping method When I add product "Expensive Jacket" to the cart Then my cart estimated shipping cost should be "$200.00" @ui @api - Scenario: Seeing estimated shipping cost that handle cheap goods + Scenario: Seeing valid estimated shipping cost for the cart with a value under maximum price configured on the shipping method When I add product "Cheap Jacket" to the cart Then my cart estimated shipping cost should be "$2.00" diff --git a/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_total_weight.feature b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_total_weight.feature index 2fc88a41e9..7bd8c79dcb 100644 --- a/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_total_weight.feature +++ b/features/shipping/applying_shipping_method_rules/seeing_estimated_shipping_cost_based_on_total_weight.feature @@ -16,12 +16,12 @@ Feature: Seeing estimated shipping costs based on total weight And this shipping method is only available for orders with a total weight less or equal to 1.0 And I am a logged in customer - @ui - Scenario: Seeing estimated shipping cost that handle heavy goods + @ui @api + Scenario: Seeing valid estimated shipping cost for the cart with a total weight over minimum total weight configured on the shipping method When I add product "Jacket for the Lochness Monster" to the cart Then my cart estimated shipping cost should be "$200.00" - @ui - Scenario: Seeing estimated shipping cost that handle cheap goods + @ui @api + Scenario: Seeing valid estimated shipping cost for the cart with a total weight under maximum total weight configured on the shipping method When I add product "T-Shirt for Tinkerbell" to the cart Then my cart estimated shipping cost should be "$2.00" diff --git a/src/Sylius/Behat/Context/Api/Shop/CartContext.php b/src/Sylius/Behat/Context/Api/Shop/CartContext.php index 58ad0b3318..2869eca07d 100644 --- a/src/Sylius/Behat/Context/Api/Shop/CartContext.php +++ b/src/Sylius/Behat/Context/Api/Shop/CartContext.php @@ -547,7 +547,7 @@ final class CartContext implements Context /** * @Then /^my cart shipping total should be ("[^"]+")$/ * @Then I should not see shipping total for my cart - * @Then /^my cart estimated shipping cost should be ("[^"]+")/ + * @Then /^my cart estimated shipping cost should be ("[^"]+")$/ */ public function myCartShippingFeeShouldBe(int $shippingTotal = 0): void { From 04ab77c3ad75d6f2879e0486d167f91e02b2a52b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Chru=C5=9Bciel?= Date: Tue, 15 Mar 2022 15:34:52 +0100 Subject: [PATCH 05/11] [Core][Configuration] Add possibilty to change priorities based on configuration --- .../DependencyInjection/Configuration.php | 1 + .../SyliusCoreExtension.php | 6 ++ .../SyliusCoreConfigurationTest.php | 63 +++++++++++++++++++ .../SyliusCoreExtensionTest.php | 30 +++++++++ 4 files changed, 100 insertions(+) create mode 100644 src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php index 2a945ceafc..2850f310f0 100644 --- a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php @@ -49,6 +49,7 @@ final class Configuration implements ConfigurationInterface ->children() ->scalarNode('driver')->defaultValue(SyliusResourceBundle::DRIVER_DOCTRINE_ORM)->end() ->booleanNode('prepend_doctrine_migrations')->defaultTrue()->end() + ->booleanNode('bring_back_previous_order_processing_priorities')->defaultFalse()->end() // Think about better naming :) ->end() ; diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php index bdcac7319a..e37cf40578 100644 --- a/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php @@ -57,6 +57,12 @@ final class SyliusCoreExtension extends AbstractResourceExtension implements Pre if ('test' === $env || 'test_cached' === $env) { $loader->load('test_services.xml'); } + + if ($config['bring_back_previous_order_processing_priorities']) { + $definition = $container->getDefinition('sylius.order_processing.order_prices_recalculator'); + $definition->clearTag('sylius.order_processor'); + $definition->addTag('sylius.order_processor', ['priority' => 40]); + } } public function prepend(ContainerBuilder $container): void diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php new file mode 100644 index 0000000000..c6ea6275e5 --- /dev/null +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php @@ -0,0 +1,63 @@ +assertProcessedConfigurationEquals( + [[]], + ['bring_back_previous_order_processing_priorities' => false], + 'bring_back_previous_order_processing_priorities', + ); + } + + /** + * @test + */ + public function it_allows_to_define_that_previous_priorities_should_be_brought_back_for_order_processing(): void + { + $this->assertProcessedConfigurationEquals( + [['bring_back_previous_order_processing_priorities' => true]], + ['bring_back_previous_order_processing_priorities' => true], + 'bring_back_previous_order_processing_priorities', + ); + } + + /** + * @test + */ + public function it_does_not_allow_to_define_previous_priorities_with_values_other_then_bool(): void + { + $this->assertConfigurationIsInvalid( + [['bring_back_previous_order_processing_priorities' => 'yolo']], + 'Invalid type for path "sylius_core.bring_back_previous_order_processing_priorities". Expected "bool", but got "string".', + ); + } + + protected function getConfiguration(): Configuration + { + return new Configuration(); + } +} diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php index a51f486477..0757ecf573 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php @@ -15,11 +15,41 @@ namespace Sylius\Bundle\CoreBundle\Tests\DependencyInjection; use Doctrine\Bundle\MigrationsBundle\DependencyInjection\DoctrineMigrationsExtension; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; +use Matthias\SymfonyDependencyInjectionTest\PhpUnit\DefinitionHasTagConstraint; use Sylius\Bundle\CoreBundle\DependencyInjection\SyliusCoreExtension; use SyliusLabs\DoctrineMigrationsExtraBundle\DependencyInjection\SyliusLabsDoctrineMigrationsExtraExtension; +use Symfony\Component\VarDumper\VarDumper; final class SyliusCoreExtensionTest extends AbstractExtensionTestCase { + /** + * @test + */ + public function it_brings_back_previous_order_processing_priorities(): void + { + $this->container->setParameter('kernel.environment', 'dev'); + + $this->load(['bring_back_previous_order_processing_priorities' => true]); + +// $this->assertContainerBuilderHasServiceDefinitionWithTag( +// 'sylius.order_processing.order_shipment_processor', +// 'sylius.order_processor', +// ['priority' => 50] +// ); + + $this->assertContainerBuilderHasServiceDefinitionWithTag( + 'sylius.order_processing.order_prices_recalculator', + 'sylius.order_processor', + ['priority' => 40] + ); + + $this->assertContainerBuilderHasServiceDefinitionWithTag( + 'sylius.order_processing.order_prices_recalculator', + 'sylius.order_processor', + ['priority' => 50] + ); + } + /** * @test */ From 225557e9c7f2eb01934c88503ce997049a34859b Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Wed, 16 Mar 2022 08:37:11 +0100 Subject: [PATCH 06/11] [Core][Configuration] Rename flag that changes priorities in order processing --- .../DependencyInjection/Configuration.php | 2 +- .../SyliusCoreExtension.php | 30 ++++++++++++++--- .../SyliusCoreConfigurationTest.php | 14 ++++---- .../SyliusCoreExtensionTest.php | 32 ++++++++++--------- 4 files changed, 51 insertions(+), 27 deletions(-) diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php index 2850f310f0..eaf2d82b5c 100644 --- a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php @@ -49,7 +49,7 @@ final class Configuration implements ConfigurationInterface ->children() ->scalarNode('driver')->defaultValue(SyliusResourceBundle::DRIVER_DOCTRINE_ORM)->end() ->booleanNode('prepend_doctrine_migrations')->defaultTrue()->end() - ->booleanNode('bring_back_previous_order_processing_priorities')->defaultFalse()->end() // Think about better naming :) + ->booleanNode('shipment_processor_before_prices_recalculator')->defaultFalse()->end() ->end() ; diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php index e37cf40578..4491e7b74b 100644 --- a/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php @@ -16,6 +16,7 @@ namespace Sylius\Bundle\CoreBundle\DependencyInjection; use Sylius\Bundle\ResourceBundle\DependencyInjection\Extension\AbstractResourceExtension; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; @@ -58,10 +59,11 @@ final class SyliusCoreExtension extends AbstractResourceExtension implements Pre $loader->load('test_services.xml'); } - if ($config['bring_back_previous_order_processing_priorities']) { - $definition = $container->getDefinition('sylius.order_processing.order_prices_recalculator'); - $definition->clearTag('sylius.order_processor'); - $definition->addTag('sylius.order_processor', ['priority' => 40]); + if ($config['shipment_processor_before_prices_recalculator']) { + $this->switchOrderProcessorsPriorities( + $container->getDefinition('sylius.order_processing.order_shipment_processor'), + $container->getDefinition('sylius.order_processing.order_prices_recalculator') + ); } } @@ -141,4 +143,24 @@ final class SyliusCoreExtension extends AbstractResourceExtension implements Pre ], ]); } + + private function switchOrderProcessorsPriorities( + Definition $firstServiceDefinition, + Definition $secondServiceDefinition + ) { + $firstServicePriority = $firstServiceDefinition->getTag('sylius.order_processor')[0]['priority']; + $secondServicePriority = $secondServiceDefinition->getTag('sylius.order_processor')[0]['priority']; + + $firstServiceDefinition->clearTag('sylius.order_processor'); + $secondServiceDefinition->clearTag('sylius.order_processor'); + + $firstServiceDefinition->addTag( + 'sylius.order_processor', + ['priority' => $secondServicePriority] + ); + $secondServiceDefinition->addTag( + 'sylius.order_processor', + ['priority' => $firstServicePriority] + ); + } } diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php index c6ea6275e5..c29d06a4e6 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php @@ -28,8 +28,8 @@ final class SyliusCoreConfigurationTest extends TestCase { $this->assertProcessedConfigurationEquals( [[]], - ['bring_back_previous_order_processing_priorities' => false], - 'bring_back_previous_order_processing_priorities', + ['shipment_processor_before_prices_recalculator' => false], + 'shipment_processor_before_prices_recalculator', ); } @@ -39,9 +39,9 @@ final class SyliusCoreConfigurationTest extends TestCase public function it_allows_to_define_that_previous_priorities_should_be_brought_back_for_order_processing(): void { $this->assertProcessedConfigurationEquals( - [['bring_back_previous_order_processing_priorities' => true]], - ['bring_back_previous_order_processing_priorities' => true], - 'bring_back_previous_order_processing_priorities', + [['shipment_processor_before_prices_recalculator' => true]], + ['shipment_processor_before_prices_recalculator' => true], + 'shipment_processor_before_prices_recalculator', ); } @@ -51,8 +51,8 @@ final class SyliusCoreConfigurationTest extends TestCase public function it_does_not_allow_to_define_previous_priorities_with_values_other_then_bool(): void { $this->assertConfigurationIsInvalid( - [['bring_back_previous_order_processing_priorities' => 'yolo']], - 'Invalid type for path "sylius_core.bring_back_previous_order_processing_priorities". Expected "bool", but got "string".', + [['shipment_processor_before_prices_recalculator' => 'yolo']], + 'Invalid type for path "sylius_core.shipment_processor_before_prices_recalculator". Expected "bool", but got "string".', ); } diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php index 0757ecf573..005a245bd2 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php @@ -29,24 +29,26 @@ final class SyliusCoreExtensionTest extends AbstractExtensionTestCase { $this->container->setParameter('kernel.environment', 'dev'); - $this->load(['bring_back_previous_order_processing_priorities' => true]); + $this->load(['shipment_processor_before_prices_recalculator' => true]); -// $this->assertContainerBuilderHasServiceDefinitionWithTag( -// 'sylius.order_processing.order_shipment_processor', -// 'sylius.order_processor', -// ['priority' => 50] -// ); - - $this->assertContainerBuilderHasServiceDefinitionWithTag( - 'sylius.order_processing.order_prices_recalculator', - 'sylius.order_processor', - ['priority' => 40] + $this->assertThat( + $this->container->findDefinition('sylius.order_processing.order_prices_recalculator'), + new DefinitionHasTagConstraint('sylius.order_processor', ['priority' => 40]) ); - $this->assertContainerBuilderHasServiceDefinitionWithTag( - 'sylius.order_processing.order_prices_recalculator', - 'sylius.order_processor', - ['priority' => 50] + $this->assertThat( + $this->container->findDefinition('sylius.order_processing.order_prices_recalculator'), + $this->logicalNot(new DefinitionHasTagConstraint('sylius.order_processor', ['priority' => 50])) + ); + + $this->assertThat( + $this->container->findDefinition('sylius.order_processing.order_shipment_processor'), + new DefinitionHasTagConstraint('sylius.order_processor', ['priority' => 50]) + ); + + $this->assertThat( + $this->container->findDefinition('sylius.order_processing.order_shipment_processor'), + $this->logicalNot(new DefinitionHasTagConstraint('sylius.order_processor', ['priority' => 40])) ); } From 12ba23573f24c125209a69885953a207398ff4c1 Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Wed, 16 Mar 2022 09:38:40 +0100 Subject: [PATCH 07/11] [Core][Configuration] Fix SyliusCoreConfigurationTest --- .../SyliusCoreConfigurationTest.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php index c29d06a4e6..d832c98d29 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php @@ -13,9 +13,11 @@ declare(strict_types=1); namespace Sylius\Bundle\CoreBundle\Tests\DependencyInjection; +use Matthias\SymfonyConfigTest\Partial\PartialProcessor; use Matthias\SymfonyConfigTest\PhpUnit\ConfigurationTestCaseTrait; use PHPUnit\Framework\TestCase; use Sylius\Bundle\CoreBundle\DependencyInjection\Configuration; +use Symfony\Component\Config\Definition\Exception\InvalidTypeException; final class SyliusCoreConfigurationTest extends TestCase { @@ -50,9 +52,12 @@ final class SyliusCoreConfigurationTest extends TestCase */ public function it_does_not_allow_to_define_previous_priorities_with_values_other_then_bool(): void { - $this->assertConfigurationIsInvalid( - [['shipment_processor_before_prices_recalculator' => 'yolo']], - 'Invalid type for path "sylius_core.shipment_processor_before_prices_recalculator". Expected "bool", but got "string".', + $this->expectException(InvalidTypeException::class); + + (new PartialProcessor())->processConfiguration( + $this->getConfiguration(), + 'shipment_processor_before_prices_recalculator', + [['shipment_processor_before_prices_recalculator' => 'yolo']] ); } From 3333d077aca47ce0f31333c01ce6fdd9b3caa833 Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Wed, 16 Mar 2022 12:33:35 +0100 Subject: [PATCH 08/11] [Core][Configuration] Refactor test cases --- .../SyliusCoreConfigurationTest.php | 12 ++------ .../SyliusCoreExtensionTest.php | 29 +++++-------------- 2 files changed, 10 insertions(+), 31 deletions(-) diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php index d832c98d29..baa7458518 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php @@ -23,9 +23,7 @@ final class SyliusCoreConfigurationTest extends TestCase { use ConfigurationTestCaseTrait; - /** - * @test - */ + /** @test */ public function it_does_not_define_that_previous_priorities_should_be_brought_back_for_order_processing(): void { $this->assertProcessedConfigurationEquals( @@ -35,9 +33,7 @@ final class SyliusCoreConfigurationTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function it_allows_to_define_that_previous_priorities_should_be_brought_back_for_order_processing(): void { $this->assertProcessedConfigurationEquals( @@ -47,9 +43,7 @@ final class SyliusCoreConfigurationTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function it_does_not_allow_to_define_previous_priorities_with_values_other_then_bool(): void { $this->expectException(InvalidTypeException::class); diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php index 005a245bd2..564c73c330 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php @@ -18,13 +18,10 @@ use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\DefinitionHasTagConstraint; use Sylius\Bundle\CoreBundle\DependencyInjection\SyliusCoreExtension; use SyliusLabs\DoctrineMigrationsExtraBundle\DependencyInjection\SyliusLabsDoctrineMigrationsExtraExtension; -use Symfony\Component\VarDumper\VarDumper; final class SyliusCoreExtensionTest extends AbstractExtensionTestCase { - /** - * @test - */ + /** @test */ public function it_brings_back_previous_order_processing_priorities(): void { $this->container->setParameter('kernel.environment', 'dev'); @@ -52,49 +49,37 @@ final class SyliusCoreExtensionTest extends AbstractExtensionTestCase ); } - /** - * @test - */ + /** @test */ public function it_autoconfigures_prepending_doctrine_migrations_with_proper_migrations_path_for_test_env(): void { $this->testPrependingDoctrineMigrations('test'); } - /** - * @test - */ + /** @test */ public function it_autoconfigures_prepending_doctrine_migrations_with_proper_migrations_path_for_test_cached_env(): void { $this->testPrependingDoctrineMigrations('test_cached'); } - /** - * @test - */ + /** @test */ public function it_autoconfigures_prepending_doctrine_migrations_with_proper_migrations_path_for_dev_env(): void { $this->testPrependingDoctrineMigrations('dev'); } - /** - * @test - */ + /** @test */ public function it_does_not_autoconfigure_prepending_doctrine_migrations_if_it_is_disabled_for_test_env(): void { $this->testNotPrependingDoctrineMigrations('test'); } - /** - * @test - */ + /** @test */ public function it_does_not_autoconfigure_prepending_doctrine_migrations_if_it_is_disabled_for_test_cached_env(): void { $this->testNotPrependingDoctrineMigrations('test_cached'); } - /** - * @test - */ + /** @test */ public function it_does_not_autoconfigure_prepending_doctrine_migrations_if_it_is_disabled_for_dev_env(): void { $this->testNotPrependingDoctrineMigrations('dev'); From 9fcae9e31fa9804576aa98bc6e2de2d2497ed5e1 Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Wed, 16 Mar 2022 12:54:58 +0100 Subject: [PATCH 09/11] [Core][Configuration] Rename flag that changes priorities in order processing --- UPGRADE-1.10.md | 2 +- .../DependencyInjection/Configuration.php | 2 +- .../DependencyInjection/SyliusCoreExtension.php | 2 +- .../SyliusCoreConfigurationTest.php | 14 +++++++------- .../SyliusCoreExtensionTest.php | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/UPGRADE-1.10.md b/UPGRADE-1.10.md index 829ca4a69a..1d8406b96d 100644 --- a/UPGRADE-1.10.md +++ b/UPGRADE-1.10.md @@ -17,7 +17,7 @@ sylius.order_processing.order_shipment_processor 40 Sylius\Com ... ``` -If you rely on previous priorities, you can bring them back by setting flag ``shipment_processor_before_prices_recalculator`` to ``true`` in ``config/packages/_sylius.yaml``. However, it is not recommended because new priorities fix [invalid estimated shipping costs](https://github.com/Sylius/Sylius/pull/13769). +If you rely on previous priorities, you can bring them back by setting flag ``processing_shipments_before_prices`` to ``true`` in ``config/packages/_sylius.yaml``. However, it is not recommended because new priorities fix [invalid estimated shipping costs](https://github.com/Sylius/Sylius/pull/13769). # UPGRADE FROM `v1.10.8` TO `v1.10.10` diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php index eaf2d82b5c..264317c3db 100644 --- a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php @@ -49,7 +49,7 @@ final class Configuration implements ConfigurationInterface ->children() ->scalarNode('driver')->defaultValue(SyliusResourceBundle::DRIVER_DOCTRINE_ORM)->end() ->booleanNode('prepend_doctrine_migrations')->defaultTrue()->end() - ->booleanNode('shipment_processor_before_prices_recalculator')->defaultFalse()->end() + ->booleanNode('processing_shipments_before_prices')->defaultFalse()->end() ->end() ; diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php index 4491e7b74b..a17d51c1b3 100644 --- a/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php @@ -59,7 +59,7 @@ final class SyliusCoreExtension extends AbstractResourceExtension implements Pre $loader->load('test_services.xml'); } - if ($config['shipment_processor_before_prices_recalculator']) { + if ($config['processing_shipments_before_prices']) { $this->switchOrderProcessorsPriorities( $container->getDefinition('sylius.order_processing.order_shipment_processor'), $container->getDefinition('sylius.order_processing.order_prices_recalculator') diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php index baa7458518..c71052c35a 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php @@ -28,8 +28,8 @@ final class SyliusCoreConfigurationTest extends TestCase { $this->assertProcessedConfigurationEquals( [[]], - ['shipment_processor_before_prices_recalculator' => false], - 'shipment_processor_before_prices_recalculator', + ['processing_shipments_before_prices' => false], + 'processing_shipments_before_prices', ); } @@ -37,9 +37,9 @@ final class SyliusCoreConfigurationTest extends TestCase public function it_allows_to_define_that_previous_priorities_should_be_brought_back_for_order_processing(): void { $this->assertProcessedConfigurationEquals( - [['shipment_processor_before_prices_recalculator' => true]], - ['shipment_processor_before_prices_recalculator' => true], - 'shipment_processor_before_prices_recalculator', + [['processing_shipments_before_prices' => true]], + ['processing_shipments_before_prices' => true], + 'processing_shipments_before_prices', ); } @@ -50,8 +50,8 @@ final class SyliusCoreConfigurationTest extends TestCase (new PartialProcessor())->processConfiguration( $this->getConfiguration(), - 'shipment_processor_before_prices_recalculator', - [['shipment_processor_before_prices_recalculator' => 'yolo']] + 'processing_shipments_before_prices', + [['processing_shipments_before_prices' => 'yolo']] ); } diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php index 564c73c330..079f5413d4 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php @@ -26,7 +26,7 @@ final class SyliusCoreExtensionTest extends AbstractExtensionTestCase { $this->container->setParameter('kernel.environment', 'dev'); - $this->load(['shipment_processor_before_prices_recalculator' => true]); + $this->load(['processing_shipments_before_prices' => true]); $this->assertThat( $this->container->findDefinition('sylius.order_processing.order_prices_recalculator'), From aac69ca011fd0dd92397f460fecdc5995634afdd Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Wed, 16 Mar 2022 14:04:47 +0100 Subject: [PATCH 10/11] [Core][Shipping] Update UPGRADE-1.10.md --- UPGRADE-1.10.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/UPGRADE-1.10.md b/UPGRADE-1.10.md index 1d8406b96d..4847e31b51 100644 --- a/UPGRADE-1.10.md +++ b/UPGRADE-1.10.md @@ -17,7 +17,12 @@ sylius.order_processing.order_shipment_processor 40 Sylius\Com ... ``` -If you rely on previous priorities, you can bring them back by setting flag ``processing_shipments_before_prices`` to ``true`` in ``config/packages/_sylius.yaml``. However, it is not recommended because new priorities fix [invalid estimated shipping costs](https://github.com/Sylius/Sylius/pull/13769). +If you rely on previous priorities, you can bring them back by setting flag ``sylius_core.processing_shipments_before_prices`` to ``true`` in ``config/packages/_sylius.yaml``: +```yaml +sylius_core: + processing_shipments_before_prices: true +``` +However, it is not recommended because new priorities fix [invalid estimated shipping costs](https://github.com/Sylius/Sylius/pull/13769). # UPGRADE FROM `v1.10.8` TO `v1.10.10` From d790944fe36a02d356a001c9bef4e16ae6da372a Mon Sep 17 00:00:00 2001 From: Kevin Kaniaburka Date: Wed, 16 Mar 2022 14:06:30 +0100 Subject: [PATCH 11/11] [Core][Configuration] Rename flag that changes priorities in order processing --- UPGRADE-1.10.md | 4 ++-- .../DependencyInjection/Configuration.php | 4 ++-- .../DependencyInjection/SyliusCoreExtension.php | 2 +- .../SyliusCoreConfigurationTest.php | 16 ++++++++-------- .../SyliusCoreExtensionTest.php | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/UPGRADE-1.10.md b/UPGRADE-1.10.md index 4847e31b51..e07b132908 100644 --- a/UPGRADE-1.10.md +++ b/UPGRADE-1.10.md @@ -17,10 +17,10 @@ sylius.order_processing.order_shipment_processor 40 Sylius\Com ... ``` -If you rely on previous priorities, you can bring them back by setting flag ``sylius_core.processing_shipments_before_prices`` to ``true`` in ``config/packages/_sylius.yaml``: +If you rely on previous priorities, you can bring them back by setting flag ``sylius_core.process_shipments_before_recalculating_prices`` to ``true`` in ``config/packages/_sylius.yaml``: ```yaml sylius_core: - processing_shipments_before_prices: true + process_shipments_before_recalculating_prices: true ``` However, it is not recommended because new priorities fix [invalid estimated shipping costs](https://github.com/Sylius/Sylius/pull/13769). diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php index 264317c3db..5d64ba48a2 100644 --- a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php @@ -49,7 +49,7 @@ final class Configuration implements ConfigurationInterface ->children() ->scalarNode('driver')->defaultValue(SyliusResourceBundle::DRIVER_DOCTRINE_ORM)->end() ->booleanNode('prepend_doctrine_migrations')->defaultTrue()->end() - ->booleanNode('processing_shipments_before_prices')->defaultFalse()->end() + ->booleanNode('process_shipments_before_recalculating_prices')->defaultFalse()->end() ->end() ; @@ -88,7 +88,7 @@ final class Configuration implements ConfigurationInterface ->addDefaultsIfNotSet() ->children() ->scalarNode('model')->defaultValue(AvatarImage::class)->cannotBeEmpty()->end() - ->scalarNode('interface')->defaultValue(AvatarImageInterface::class)->cannotBeEmpty()->end() + ->scalarNode('interface')->defaultValue(AvatarImageInterface::class)->cannotBeEmpty()->end() ->scalarNode('repository')->defaultValue(AvatarImageRepository::class)->cannotBeEmpty()->end() ->scalarNode('factory')->defaultValue(Factory::class)->end() ->end() diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php index a17d51c1b3..7071cae529 100644 --- a/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php @@ -59,7 +59,7 @@ final class SyliusCoreExtension extends AbstractResourceExtension implements Pre $loader->load('test_services.xml'); } - if ($config['processing_shipments_before_prices']) { + if ($config['process_shipments_before_recalculating_prices']) { $this->switchOrderProcessorsPriorities( $container->getDefinition('sylius.order_processing.order_shipment_processor'), $container->getDefinition('sylius.order_processing.order_prices_recalculator') diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php index c71052c35a..362daa3e0a 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreConfigurationTest.php @@ -24,12 +24,12 @@ final class SyliusCoreConfigurationTest extends TestCase use ConfigurationTestCaseTrait; /** @test */ - public function it_does_not_define_that_previous_priorities_should_be_brought_back_for_order_processing(): void + public function it_does_not_bring_back_previous_priorities_for_order_processing_by_default(): void { $this->assertProcessedConfigurationEquals( [[]], - ['processing_shipments_before_prices' => false], - 'processing_shipments_before_prices', + ['process_shipments_before_recalculating_prices' => false], + 'process_shipments_before_recalculating_prices', ); } @@ -37,9 +37,9 @@ final class SyliusCoreConfigurationTest extends TestCase public function it_allows_to_define_that_previous_priorities_should_be_brought_back_for_order_processing(): void { $this->assertProcessedConfigurationEquals( - [['processing_shipments_before_prices' => true]], - ['processing_shipments_before_prices' => true], - 'processing_shipments_before_prices', + [['process_shipments_before_recalculating_prices' => true]], + ['process_shipments_before_recalculating_prices' => true], + 'process_shipments_before_recalculating_prices', ); } @@ -50,8 +50,8 @@ final class SyliusCoreConfigurationTest extends TestCase (new PartialProcessor())->processConfiguration( $this->getConfiguration(), - 'processing_shipments_before_prices', - [['processing_shipments_before_prices' => 'yolo']] + 'process_shipments_before_recalculating_prices', + [['process_shipments_before_recalculating_prices' => 'yolo']] ); } diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php index 079f5413d4..64db9c06b3 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/SyliusCoreExtensionTest.php @@ -26,7 +26,7 @@ final class SyliusCoreExtensionTest extends AbstractExtensionTestCase { $this->container->setParameter('kernel.environment', 'dev'); - $this->load(['processing_shipments_before_prices' => true]); + $this->load(['process_shipments_before_recalculating_prices' => true]); $this->assertThat( $this->container->findDefinition('sylius.order_processing.order_prices_recalculator'),