From 90de95cd2f97d18199e2700660b975251fedb570 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 10 Apr 2018 11:56:23 +0200 Subject: [PATCH] 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,