From c742822b1505425326c34a13fefebff11ae0f0d2 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 12 Sep 2017 12:21:33 +0200 Subject: [PATCH 1/4] [Behat] Default shipping method selection fixes scenario --- ...selected_based_on_shipping_address.feature | 41 +++++++++++++++++++ .../Shop/Checkout/CheckoutShippingContext.php | 8 ++++ .../Page/Shop/Checkout/SelectShippingPage.php | 19 +++++++++ .../Checkout/SelectShippingPageInterface.php | 5 +++ 4 files changed, 73 insertions(+) create mode 100644 features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature diff --git a/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature b/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature new file mode 100644 index 00000000000..670817e510f --- /dev/null +++ b/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature @@ -0,0 +1,41 @@ +@checkout +Feature: Seeing shipping methods which category is same as category of all my units + In order to select correct shipping method for my order + As a Customer + I want to be able to choose only shipping methods that match shipping category of all my items + + Background: + Given the store operates on a channel named "Web" + And the store has a product "Star Trek Ship" priced at "$19.99" + And the store ships to "United Kingdom" + And the store ships to "United States" + And the store has a zone "United Kingdom" with code "UK" + And this zone has the "United Kingdom" country member + And the store has a zone "United States" with code "US" + And this zone has the "United States" country member + And the store has "DHL" shipping method with "$10.00" fee within the "US" zone + And the store has "FedEx" shipping method with "$20.00" fee within the "UK" zone + And I am a logged in customer + + @ui + Scenario: Seeing default shipping method selected based on country from shipping address + Given I have product "Star Trek Ship" in the cart + When I am at the checkout addressing step + And I specify the shipping address as "Ankh Morpork", "Frost Alley", "90210", "United States" for "Jon Snow" + And I complete the addressing step + Then I should be on the checkout shipping step + And I should see selected "DHL" shipping method + And I should not see "FedEx" shipping method + + @ui + Scenario: Seeing default shipping method selected based on country from shipping address after readdressing + Given I have product "Star Trek Ship" in the cart + When I am at the checkout addressing step + And I specify the shipping address as "Ankh Morpork", "Frost Alley", "90210", "United States" for "Jon Snow" + And I complete the addressing step + And I decide to change my address + And I specify the shipping address as "Ankh Morpork", "Frost Alley", "90210", "United Kingdom" for "Jon Snow" + And I complete the addressing step + Then I should be on the checkout shipping step + And I should see selected "FedEx" shipping method + And I should not see "DHL" shipping method diff --git a/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutShippingContext.php b/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutShippingContext.php index f6ecc42343e..c1cd95b81ba 100644 --- a/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutShippingContext.php +++ b/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutShippingContext.php @@ -191,6 +191,14 @@ public function iShouldSeeShippingMethod($shippingMethodName) Assert::true($this->selectShippingPage->hasShippingMethod($shippingMethodName)); } + /** + * @Then I should see selected :shippingMethodName shipping method + */ + public function iShouldSeeSelectedShippingMethod($shippingMethodName) + { + Assert::same($this->selectShippingPage->getSelectedShippingMethodName(), $shippingMethodName); + } + /** * @Then I should not see :shippingMethodName shipping method */ diff --git a/src/Sylius/Behat/Page/Shop/Checkout/SelectShippingPage.php b/src/Sylius/Behat/Page/Shop/Checkout/SelectShippingPage.php index e1f33820c67..42a1b6876ed 100644 --- a/src/Sylius/Behat/Page/Shop/Checkout/SelectShippingPage.php +++ b/src/Sylius/Behat/Page/Shop/Checkout/SelectShippingPage.php @@ -14,6 +14,7 @@ namespace Sylius\Behat\Page\Shop\Checkout; use Behat\Mink\Driver\Selenium2Driver; +use Behat\Mink\Element\NodeElement; use Behat\Mink\Exception\ElementNotFoundException; use Sylius\Behat\Page\SymfonyPage; @@ -57,6 +58,24 @@ public function getShippingMethods() return $shippingMethods; } + /** + * {@inheritdoc} + */ + public function getSelectedShippingMethodName(): ?string + { + $shippingMethods = $this->getSession()->getPage()->findAll('css', '#sylius-shipping-methods .item'); + + /** @var NodeElement $shippingMethod */ + foreach ($shippingMethods as $shippingMethod) { + if (null !== $shippingMethod->find('css', 'input:checked')) { + return $shippingMethod->find('css', '.content label')->getText(); + } + } + + return null; + } + + /** * {@inheritdoc} */ diff --git a/src/Sylius/Behat/Page/Shop/Checkout/SelectShippingPageInterface.php b/src/Sylius/Behat/Page/Shop/Checkout/SelectShippingPageInterface.php index 40ee608e857..9a3185acdb8 100644 --- a/src/Sylius/Behat/Page/Shop/Checkout/SelectShippingPageInterface.php +++ b/src/Sylius/Behat/Page/Shop/Checkout/SelectShippingPageInterface.php @@ -27,6 +27,11 @@ public function selectShippingMethod($shippingMethod); */ public function getShippingMethods(); + /** + * @return string|null + */ + public function getSelectedShippingMethodName(): ?string; + /** * @return bool */ From e4cb36608321c8b54a78cc069a2bc02acf8328f9 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 12 Sep 2017 13:12:10 +0200 Subject: [PATCH 2/4] [Core] Resolve default shipping method also by zone --- .../CoreBundle/Resources/config/services.xml | 1 + .../DefaultShippingMethodResolver.php | 36 ++++++++-- .../DefaultShippingMethodResolverSpec.php | 66 ++++++++++++++++--- 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml index 426b016e223..cdebe83c23d 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml @@ -70,6 +70,7 @@ + diff --git a/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php b/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php index e2f0da66d47..9ea06d5e47e 100644 --- a/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php +++ b/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php @@ -13,6 +13,9 @@ namespace Sylius\Component\Core\Resolver; +use Sylius\Component\Addressing\Matcher\ZoneMatcherInterface; +use Sylius\Component\Addressing\Model\ZoneInterface; +use Sylius\Component\Core\Model\AddressInterface; use Sylius\Component\Core\Model\ChannelInterface; use Sylius\Component\Core\Model\ShipmentInterface as CoreShipmentInterface; use Sylius\Component\Core\Repository\ShippingMethodRepositoryInterface; @@ -29,12 +32,21 @@ class DefaultShippingMethodResolver implements DefaultShippingMethodResolverInte */ private $shippingMethodRepository; + /** + * @var ZoneMatcherInterface + */ + private $zoneMatcher; + /** * @param ShippingMethodRepositoryInterface $shippingMethodRepository + * @param ZoneMatcherInterface $zoneMatcher */ - public function __construct(ShippingMethodRepositoryInterface $shippingMethodRepository) - { + public function __construct( + ShippingMethodRepositoryInterface $shippingMethodRepository, + ZoneMatcherInterface $zoneMatcher + ) { $this->shippingMethodRepository = $shippingMethodRepository; + $this->zoneMatcher = $zoneMatcher; } /** @@ -45,14 +57,30 @@ public function getDefaultShippingMethod(ShipmentInterface $shipment): ShippingM /** @var CoreShipmentInterface $shipment */ Assert::isInstanceOf($shipment, CoreShipmentInterface::class); + $order = $shipment->getOrder(); + /** @var ChannelInterface $channel */ - $channel = $shipment->getOrder()->getChannel(); + $channel = $order->getChannel(); + /** @var AddressInterface $shippingAddress */ + $shippingAddress = $order->getShippingAddress(); - $shippingMethods = $this->shippingMethodRepository->findEnabledForChannel($channel); + $shippingMethods = $this->getShippingMethods($channel, $shippingAddress); if (empty($shippingMethods)) { throw new UnresolvedDefaultShippingMethodException(); } return $shippingMethods[0]; } + + private function getShippingMethods(ChannelInterface $channel, ?AddressInterface $address): array + { + if (null === $address) { + return $this->shippingMethodRepository->findEnabledForChannel($channel); + } + + /** @var ZoneInterface[] $zones */ + $zones = $this->zoneMatcher->matchAll($address); + + return $this->shippingMethodRepository->findEnabledForZonesAndChannel($zones, $channel); + } } diff --git a/src/Sylius/Component/Core/spec/Resolver/DefaultShippingMethodResolverSpec.php b/src/Sylius/Component/Core/spec/Resolver/DefaultShippingMethodResolverSpec.php index 6ddc3d284f6..da5e0fb0cce 100644 --- a/src/Sylius/Component/Core/spec/Resolver/DefaultShippingMethodResolverSpec.php +++ b/src/Sylius/Component/Core/spec/Resolver/DefaultShippingMethodResolverSpec.php @@ -14,6 +14,10 @@ namespace spec\Sylius\Component\Core\Resolver; use PhpSpec\ObjectBehavior; +use Prophecy\Argument; +use Sylius\Component\Addressing\Matcher\ZoneMatcherInterface; +use Sylius\Component\Addressing\Model\ZoneInterface; +use Sylius\Component\Core\Model\AddressInterface; use Sylius\Component\Core\Model\ChannelInterface; use Sylius\Component\Core\Model\OrderInterface; use Sylius\Component\Core\Model\ShipmentInterface; @@ -25,9 +29,11 @@ final class DefaultShippingMethodResolverSpec extends ObjectBehavior { - function let(ShippingMethodRepositoryInterface $shippingMethodRepository): void - { - $this->beConstructedWith($shippingMethodRepository); + function let( + ShippingMethodRepositoryInterface $shippingMethodRepository, + ZoneMatcherInterface $zoneMatcher + ): void { + $this->beConstructedWith($shippingMethodRepository, $zoneMatcher); } function it_implements_a_default_shipping_method_resolver_interface(): void @@ -35,16 +41,21 @@ function it_implements_a_default_shipping_method_resolver_interface(): void $this->shouldImplement(DefaultShippingMethodResolverInterface::class); } - function it_returns_first_enabled_shipping_method_from_shipment_order_channel_as_default( + function it_returns_first_enabled_shipping_method_from_shipment_order_channel_if_there_is_not_shipping_address( ChannelInterface $channel, OrderInterface $order, ShipmentInterface $shipment, ShippingMethodInterface $firstShippingMethod, ShippingMethodInterface $secondShippingMethod, - ShippingMethodRepositoryInterface $shippingMethodRepository + ShippingMethodRepositoryInterface $shippingMethodRepository, + ZoneMatcherInterface $zoneMatcher ): void { $shipment->getOrder()->willReturn($order); + $order->getChannel()->willReturn($channel); + $order->getShippingAddress()->willReturn(null); + + $zoneMatcher->matchAll(Argument::any())->shouldNotBeCalled(); $shippingMethodRepository ->findEnabledForChannel($channel) @@ -54,17 +65,54 @@ function it_returns_first_enabled_shipping_method_from_shipment_order_channel_as $this->getDefaultShippingMethod($shipment)->shouldReturn($firstShippingMethod); } - function it_throws_an_exception_if_there_is_no_enabled_shipping_methods( - ShippingMethodRepositoryInterface $shippingMethodRepository, + function it_returns_first_enabled_shipping_method_matched_by_order_channel_and_shipping_address_zone( + AddressInterface $shippingAddress, + ChannelInterface $channel, + OrderInterface $order, ShipmentInterface $shipment, + ShippingMethodInterface $firstShippingMethod, + ShippingMethodInterface $secondShippingMethod, + ShippingMethodRepositoryInterface $shippingMethodRepository, + ZoneInterface $firstZone, + ZoneInterface $secondZone, + ZoneMatcherInterface $zoneMatcher + ): void { + $shipment->getOrder()->willReturn($order); + + $order->getChannel()->willReturn($channel); + $order->getShippingAddress()->willReturn($shippingAddress); + + $zoneMatcher->matchAll($shippingAddress)->willReturn([$firstZone, $secondZone]); + + $shippingMethodRepository + ->findEnabledForZonesAndChannel([$firstZone, $secondZone], $channel) + ->willReturn([$firstShippingMethod, $secondShippingMethod]) + ; + + $this->getDefaultShippingMethod($shipment)->shouldReturn($firstShippingMethod); + } + + function it_throws_an_exception_if_there_is_no_enabled_shipping_methods_for_order_channel_and_zones( + AddressInterface $shippingAddress, ChannelInterface $channel, - OrderInterface $order + OrderInterface $order, + ShipmentInterface $shipment, + ShippingMethodRepositoryInterface $shippingMethodRepository, + ZoneInterface $firstZone, + ZoneInterface $secondZone, + ZoneMatcherInterface $zoneMatcher ): void { $shipment->getOrder()->willReturn($order); + $order->getChannel()->willReturn($channel); + $order->getShippingAddress()->willReturn($shippingAddress); + + $zoneMatcher->matchAll($shippingAddress)->willReturn([$firstZone, $secondZone]); $shippingMethodRepository - ->findEnabledForChannel($channel)->willReturn([]); + ->findEnabledForZonesAndChannel([$firstZone, $secondZone], $channel) + ->willReturn([]) + ; $this ->shouldThrow(UnresolvedDefaultShippingMethodException::class) From 68154e3d8e2fa2beb1f323af4528de880fe2ada7 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 12 Sep 2017 14:39:55 +0200 Subject: [PATCH 3/4] [Core] Control existing shipment's method during checkout --- ...selected_based_on_shipping_address.feature | 8 +-- .../config/services/order_processing.xml | 1 + .../OrderShipmentProcessor.php | 32 +++++++++- .../DefaultShippingMethodResolver.php | 6 ++ .../OrderShipmentProcessorSpec.php | 59 ++++++++++++++++++- 5 files changed, 98 insertions(+), 8 deletions(-) diff --git a/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature b/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature index 670817e510f..fdde5af7d63 100644 --- a/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature +++ b/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature @@ -20,8 +20,8 @@ Feature: Seeing shipping methods which category is same as category of all my un @ui Scenario: Seeing default shipping method selected based on country from shipping address Given I have product "Star Trek Ship" in the cart - When I am at the checkout addressing step - And I specify the shipping address as "Ankh Morpork", "Frost Alley", "90210", "United States" for "Jon Snow" + And I am at the checkout addressing step + When I specify the shipping address as "Ankh Morpork", "Frost Alley", "90210", "United States" for "Jon Snow" And I complete the addressing step Then I should be on the checkout shipping step And I should see selected "DHL" shipping method @@ -30,8 +30,8 @@ Feature: Seeing shipping methods which category is same as category of all my un @ui Scenario: Seeing default shipping method selected based on country from shipping address after readdressing Given I have product "Star Trek Ship" in the cart - When I am at the checkout addressing step - And I specify the shipping address as "Ankh Morpork", "Frost Alley", "90210", "United States" for "Jon Snow" + And I am at the checkout addressing step + When I specify the shipping address as "Ankh Morpork", "Frost Alley", "90210", "United States" for "Jon Snow" And I complete the addressing step And I decide to change my address And I specify the shipping address as "Ankh Morpork", "Frost Alley", "90210", "United Kingdom" for "Jon Snow" 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 503ba37b33d..52e29cdb77c 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/order_processing.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/order_processing.xml @@ -29,6 +29,7 @@ + diff --git a/src/Sylius/Component/Core/OrderProcessing/OrderShipmentProcessor.php b/src/Sylius/Component/Core/OrderProcessing/OrderShipmentProcessor.php index 4c0b8da8018..8c87decd25b 100644 --- a/src/Sylius/Component/Core/OrderProcessing/OrderShipmentProcessor.php +++ b/src/Sylius/Component/Core/OrderProcessing/OrderShipmentProcessor.php @@ -20,6 +20,7 @@ use Sylius\Component\Resource\Factory\FactoryInterface; use Sylius\Component\Shipping\Exception\UnresolvedDefaultShippingMethodException; use Sylius\Component\Shipping\Resolver\DefaultShippingMethodResolverInterface; +use Sylius\Component\Shipping\Resolver\ShippingMethodsResolverInterface; use Webmozart\Assert\Assert; final class OrderShipmentProcessor implements OrderProcessorInterface @@ -29,6 +30,11 @@ final class OrderShipmentProcessor implements OrderProcessorInterface */ private $defaultShippingMethodResolver; + /** + * @var ShippingMethodsResolverInterface + */ + private $shippingMethodsResolver; + /** * @var FactoryInterface */ @@ -36,13 +42,16 @@ final class OrderShipmentProcessor implements OrderProcessorInterface /** * @param DefaultShippingMethodResolverInterface $defaultShippingMethodResolver + * @param ShippingMethodsResolverInterface $shippingMethodsResolver * @param FactoryInterface $shipmentFactory */ public function __construct( DefaultShippingMethodResolverInterface $defaultShippingMethodResolver, + ShippingMethodsResolverInterface $shippingMethodsResolver, FactoryInterface $shipmentFactory ) { $this->defaultShippingMethodResolver = $defaultShippingMethodResolver; + $this->shippingMethodsResolver = $shippingMethodsResolver; $this->shipmentFactory = $shipmentFactory; } @@ -85,7 +94,7 @@ public function process(BaseOrderInterface $order): void private function getOrderShipment(OrderInterface $order): ?ShipmentInterface { if ($order->hasShipments()) { - return $order->getShipments()->first(); + return $this->getExistingShipmentWithProperMethod($order); } try { @@ -101,4 +110,25 @@ private function getOrderShipment(OrderInterface $order): ?ShipmentInterface return null; } } + + /** + * @param OrderInterface $order + * + * @return ShipmentInterface|null + */ + private function getExistingShipmentWithProperMethod(OrderInterface $order): ?ShipmentInterface + { + /** @var ShipmentInterface $shipment */ + $shipment = $order->getShipments()->first(); + + if (!in_array($shipment->getMethod(), $this->shippingMethodsResolver->getSupportedMethods($shipment))) { + try { + $shipment->setMethod($this->defaultShippingMethodResolver->getDefaultShippingMethod($shipment)); + } catch (UnresolvedDefaultShippingMethodException $exception) { + return null; + } + } + + return $shipment; + } } diff --git a/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php b/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php index 9ea06d5e47e..2249a81de13 100644 --- a/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php +++ b/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php @@ -72,6 +72,12 @@ public function getDefaultShippingMethod(ShipmentInterface $shipment): ShippingM return $shippingMethods[0]; } + /** + * @param ChannelInterface $channel + * @param AddressInterface|null $address + * + * @return array + */ private function getShippingMethods(ChannelInterface $channel, ?AddressInterface $address): array { if (null === $address) { diff --git a/src/Sylius/Component/Core/spec/OrderProcessing/OrderShipmentProcessorSpec.php b/src/Sylius/Component/Core/spec/OrderProcessing/OrderShipmentProcessorSpec.php index 43d38cd4cca..328ec3b4775 100644 --- a/src/Sylius/Component/Core/spec/OrderProcessing/OrderShipmentProcessorSpec.php +++ b/src/Sylius/Component/Core/spec/OrderProcessing/OrderShipmentProcessorSpec.php @@ -25,14 +25,16 @@ use Sylius\Component\Resource\Factory\FactoryInterface; use Sylius\Component\Shipping\Model\ShippingMethodInterface; use Sylius\Component\Shipping\Resolver\DefaultShippingMethodResolverInterface; +use Sylius\Component\Shipping\Resolver\ShippingMethodsResolverInterface; final class OrderShipmentProcessorSpec extends ObjectBehavior { function let( DefaultShippingMethodResolverInterface $defaultShippingMethodResolver, + ShippingMethodsResolverInterface $shippingMethodsResolver, FactoryInterface $shipmentFactory ): void { - $this->beConstructedWith($defaultShippingMethodResolver, $shipmentFactory); + $this->beConstructedWith($defaultShippingMethodResolver, $shippingMethodsResolver, $shipmentFactory); } function it_is_an_order_processor(): void @@ -100,16 +102,21 @@ function it_removes_shipments_and_returns_null_when_shipping_is_not_required( } function it_adds_new_item_units_to_existing_shipment( + ShippingMethodsResolverInterface $shippingMethodsResolver, OrderInterface $order, ShipmentInterface $shipment, Collection $shipments, OrderItemUnitInterface $itemUnit, OrderItemUnitInterface $itemUnitWithoutShipment, OrderItemInterface $orderItem, - ProductVariantInterface $productVariant + ProductVariantInterface $productVariant, + ShippingMethodInterface $shippingMethod ): void { $shipments->first()->willReturn($shipment); + $shipment->getMethod()->willReturn($shippingMethod); + $shippingMethodsResolver->getSupportedMethods($shipment)->willReturn([$shippingMethod]); + $orderItem->getVariant()->willReturn($productVariant); $order->isShippingRequired()->willReturn(true); @@ -131,14 +138,19 @@ function it_adds_new_item_units_to_existing_shipment( } function it_removes_units_before_adding_new_ones( + ShippingMethodsResolverInterface $shippingMethodsResolver, OrderInterface $order, ShipmentInterface $shipment, Collection $shipments, OrderItemUnitInterface $itemUnit, - OrderItemUnitInterface $itemUnitWithoutShipment + OrderItemUnitInterface $itemUnitWithoutShipment, + ShippingMethodInterface $shippingMethod ): void { $shipments->first()->willReturn($shipment); + $shipment->getMethod()->willReturn($shippingMethod); + $shippingMethodsResolver->getSupportedMethods($shipment)->willReturn([$shippingMethod]); + $order->isShippingRequired()->willReturn(true); $order->isEmpty()->willReturn(false); @@ -156,4 +168,45 @@ function it_removes_units_before_adding_new_ones( $this->process($order); } + + function it_adds_new_item_units_to_existing_shipment_and_replaces_its_method_if_its_ineligible( + DefaultShippingMethodResolverInterface $defaultShippingMethodResolver, + ShippingMethodsResolverInterface $shippingMethodsResolver, + OrderInterface $order, + ShipmentInterface $shipment, + Collection $shipments, + OrderItemUnitInterface $itemUnit, + OrderItemUnitInterface $itemUnitWithoutShipment, + OrderItemInterface $orderItem, + ProductVariantInterface $productVariant, + ShippingMethodInterface $firstShippingMethod, + ShippingMethodInterface $secondShippingMethod + ): void { + $shipments->first()->willReturn($shipment); + + $shipment->getMethod()->willReturn($firstShippingMethod); + $shippingMethodsResolver->getSupportedMethods($shipment)->willReturn([$secondShippingMethod]); + + $defaultShippingMethodResolver->getDefaultShippingMethod($shipment)->willReturn($secondShippingMethod); + $shipment->setMethod($secondShippingMethod)->shouldBeCalled(); + + $orderItem->getVariant()->willReturn($productVariant); + + $order->isShippingRequired()->willReturn(true); + + $order->getItems()->willReturn(new ArrayCollection([$orderItem->getWrappedObject()])); + + $order->isEmpty()->willReturn(false); + $order->hasShipments()->willReturn(true); + $order->getItemUnits()->willReturn(new ArrayCollection([$itemUnit->getWrappedObject(), $itemUnitWithoutShipment->getWrappedObject()])); + $order->getShipments()->willReturn($shipments); + + $itemUnit->getShipment()->willReturn($shipment); + + $shipment->getUnits()->willReturn(new ArrayCollection([])); + $shipment->addUnit($itemUnitWithoutShipment)->shouldBeCalled(); + $shipment->addUnit($itemUnit)->shouldNotBeCalled(); + + $this->process($order); + } } From 90de95cd2f97d18199e2700660b975251fedb570 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 10 Apr 2018 11:56:23 +0200 Subject: [PATCH 4/4] Avoid BC breaks in modified services --- ...selected_based_on_shipping_address.feature | 2 +- .../config/services/order_processing.xml | 2 +- .../OrderShipmentProcessor.php | 29 +++++++++---- .../DefaultShippingMethodResolver.php | 17 +++++--- .../OrderShipmentProcessorSpec.php | 41 +++++++++++++++++-- .../DefaultShippingMethodResolverSpec.php | 24 +++++++++++ 6 files changed, 96 insertions(+), 19 deletions(-) diff --git a/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature b/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature index fdde5af7d63..a69455c2375 100644 --- a/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature +++ b/features/checkout/shipping_order/seeing_default_shipping_method_selected_based_on_shipping_address.feature @@ -1,5 +1,5 @@ @checkout -Feature: Seeing shipping methods which category is same as category of all my units +Feature: Seeing default shipping method selected based on shipping address In order to select correct shipping method for my order As a Customer I want to be able to choose only shipping methods that match shipping category of all my items 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 52e29cdb77c..5d506342560 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/order_processing.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/order_processing.xml @@ -29,8 +29,8 @@ - + diff --git a/src/Sylius/Component/Core/OrderProcessing/OrderShipmentProcessor.php b/src/Sylius/Component/Core/OrderProcessing/OrderShipmentProcessor.php index 8c87decd25b..f292431efea 100644 --- a/src/Sylius/Component/Core/OrderProcessing/OrderShipmentProcessor.php +++ b/src/Sylius/Component/Core/OrderProcessing/OrderShipmentProcessor.php @@ -31,28 +31,35 @@ final class OrderShipmentProcessor implements OrderProcessorInterface private $defaultShippingMethodResolver; /** - * @var ShippingMethodsResolverInterface + * @var FactoryInterface */ - private $shippingMethodsResolver; + private $shipmentFactory; /** - * @var FactoryInterface + * @var ShippingMethodsResolverInterface|null */ - private $shipmentFactory; + private $shippingMethodsResolver; /** * @param DefaultShippingMethodResolverInterface $defaultShippingMethodResolver - * @param ShippingMethodsResolverInterface $shippingMethodsResolver * @param FactoryInterface $shipmentFactory + * @param ShippingMethodsResolverInterface|null $shippingMethodsResolver */ public function __construct( DefaultShippingMethodResolverInterface $defaultShippingMethodResolver, - ShippingMethodsResolverInterface $shippingMethodsResolver, - FactoryInterface $shipmentFactory + FactoryInterface $shipmentFactory, + ?ShippingMethodsResolverInterface $shippingMethodsResolver = null ) { $this->defaultShippingMethodResolver = $defaultShippingMethodResolver; - $this->shippingMethodsResolver = $shippingMethodsResolver; $this->shipmentFactory = $shipmentFactory; + $this->shippingMethodsResolver = $shippingMethodsResolver; + + if (2 === func_num_args() || null === $shippingMethodsResolver) { + @trigger_error( + 'Not passing ShippingMethodsResolverInterface explicitly is deprecated since 1.2 and will be prohibited in 2.0', + E_USER_DEPRECATED + ); + } } /** @@ -121,7 +128,11 @@ private function getExistingShipmentWithProperMethod(OrderInterface $order): ?Sh /** @var ShipmentInterface $shipment */ $shipment = $order->getShipments()->first(); - if (!in_array($shipment->getMethod(), $this->shippingMethodsResolver->getSupportedMethods($shipment))) { + if (null === $this->shippingMethodsResolver) { + return $shipment; + } + + if (!in_array($shipment->getMethod(), $this->shippingMethodsResolver->getSupportedMethods($shipment), true)) { try { $shipment->setMethod($this->defaultShippingMethodResolver->getDefaultShippingMethod($shipment)); } catch (UnresolvedDefaultShippingMethodException $exception) { diff --git a/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php b/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php index 2249a81de13..6331ff2c4cb 100644 --- a/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php +++ b/src/Sylius/Component/Core/Resolver/DefaultShippingMethodResolver.php @@ -33,20 +33,27 @@ class DefaultShippingMethodResolver implements DefaultShippingMethodResolverInte private $shippingMethodRepository; /** - * @var ZoneMatcherInterface + * @var ZoneMatcherInterface|null */ private $zoneMatcher; /** * @param ShippingMethodRepositoryInterface $shippingMethodRepository - * @param ZoneMatcherInterface $zoneMatcher + * @param ZoneMatcherInterface|null $zoneMatcher */ public function __construct( ShippingMethodRepositoryInterface $shippingMethodRepository, - ZoneMatcherInterface $zoneMatcher + ?ZoneMatcherInterface $zoneMatcher = null ) { $this->shippingMethodRepository = $shippingMethodRepository; $this->zoneMatcher = $zoneMatcher; + + if (1 === func_num_args() || null === $zoneMatcher) { + @trigger_error( + 'Not passing ZoneMatcherInterface explicitly is deprecated since 1.2 and will be prohibited in 2.0', + E_USER_DEPRECATED + ); + } } /** @@ -76,11 +83,11 @@ public function getDefaultShippingMethod(ShipmentInterface $shipment): ShippingM * @param ChannelInterface $channel * @param AddressInterface|null $address * - * @return array + * @return array|ShippingMethodInterface[] */ private function getShippingMethods(ChannelInterface $channel, ?AddressInterface $address): array { - if (null === $address) { + if (null === $address || null === $this->zoneMatcher) { return $this->shippingMethodRepository->findEnabledForChannel($channel); } diff --git a/src/Sylius/Component/Core/spec/OrderProcessing/OrderShipmentProcessorSpec.php b/src/Sylius/Component/Core/spec/OrderProcessing/OrderShipmentProcessorSpec.php index 328ec3b4775..6d895abc0b3 100644 --- a/src/Sylius/Component/Core/spec/OrderProcessing/OrderShipmentProcessorSpec.php +++ b/src/Sylius/Component/Core/spec/OrderProcessing/OrderShipmentProcessorSpec.php @@ -31,10 +31,10 @@ final class OrderShipmentProcessorSpec extends ObjectBehavior { function let( DefaultShippingMethodResolverInterface $defaultShippingMethodResolver, - ShippingMethodsResolverInterface $shippingMethodsResolver, - FactoryInterface $shipmentFactory + FactoryInterface $shipmentFactory, + ShippingMethodsResolverInterface $shippingMethodsResolver ): void { - $this->beConstructedWith($defaultShippingMethodResolver, $shippingMethodsResolver, $shipmentFactory); + $this->beConstructedWith($defaultShippingMethodResolver, $shipmentFactory, $shippingMethodsResolver); } function it_is_an_order_processor(): void @@ -137,6 +137,41 @@ function it_adds_new_item_units_to_existing_shipment( $this->process($order); } + function it_adds_new_item_units_to_existing_shipment_without_checking_methods_if_shipping_methods_resolver_is_not_used( + DefaultShippingMethodResolverInterface $defaultShippingMethodResolver, + FactoryInterface $shipmentFactory, + OrderInterface $order, + ShipmentInterface $shipment, + Collection $shipments, + OrderItemUnitInterface $itemUnit, + OrderItemUnitInterface $itemUnitWithoutShipment, + OrderItemInterface $orderItem, + ProductVariantInterface $productVariant + ): void { + $this->beConstructedWith($defaultShippingMethodResolver, $shipmentFactory); + + $shipments->first()->willReturn($shipment); + + $orderItem->getVariant()->willReturn($productVariant); + + $order->isShippingRequired()->willReturn(true); + + $order->getItems()->willReturn(new ArrayCollection([$orderItem->getWrappedObject()])); + + $order->isEmpty()->willReturn(false); + $order->hasShipments()->willReturn(true); + $order->getItemUnits()->willReturn(new ArrayCollection([$itemUnit->getWrappedObject(), $itemUnitWithoutShipment->getWrappedObject()])); + $order->getShipments()->willReturn($shipments); + + $itemUnit->getShipment()->willReturn($shipment); + + $shipment->getUnits()->willReturn(new ArrayCollection([])); + $shipment->addUnit($itemUnitWithoutShipment)->shouldBeCalled(); + $shipment->addUnit($itemUnit)->shouldNotBeCalled(); + + $this->process($order); + } + function it_removes_units_before_adding_new_ones( ShippingMethodsResolverInterface $shippingMethodsResolver, OrderInterface $order, diff --git a/src/Sylius/Component/Core/spec/Resolver/DefaultShippingMethodResolverSpec.php b/src/Sylius/Component/Core/spec/Resolver/DefaultShippingMethodResolverSpec.php index da5e0fb0cce..9e2915bab45 100644 --- a/src/Sylius/Component/Core/spec/Resolver/DefaultShippingMethodResolverSpec.php +++ b/src/Sylius/Component/Core/spec/Resolver/DefaultShippingMethodResolverSpec.php @@ -65,6 +65,30 @@ function it_returns_first_enabled_shipping_method_from_shipment_order_channel_if $this->getDefaultShippingMethod($shipment)->shouldReturn($firstShippingMethod); } + function it_returns_first_enabled_shipping_method_from_shipment_order_channel_if_zone_matcher_is_not_used( + AddressInterface $shippingAddress, + ChannelInterface $channel, + OrderInterface $order, + ShipmentInterface $shipment, + ShippingMethodInterface $firstShippingMethod, + ShippingMethodInterface $secondShippingMethod, + ShippingMethodRepositoryInterface $shippingMethodRepository + ): void { + $this->beConstructedWith($shippingMethodRepository); + + $shipment->getOrder()->willReturn($order); + + $order->getChannel()->willReturn($channel); + $order->getShippingAddress()->willReturn($shippingAddress); + + $shippingMethodRepository + ->findEnabledForChannel($channel) + ->willReturn([$firstShippingMethod, $secondShippingMethod]) + ; + + $this->getDefaultShippingMethod($shipment)->shouldReturn($firstShippingMethod); + } + function it_returns_first_enabled_shipping_method_matched_by_order_channel_and_shipping_address_zone( AddressInterface $shippingAddress, ChannelInterface $channel,