From 2a1c1676a0bad065e9f4bb2d05f37c07bb592d33 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Thu, 19 May 2016 11:21:27 +0200 Subject: [PATCH 1/2] [Core] Remove promotion and tax adjustments before recalculation --- .../CoreBundle/Resources/config/services.xml | 5 ++ .../OrderProcessing/OrderRecalculator.php | 9 +++ .../Core/Remover/AdjustmentsRemover.php | 55 ++++++++++++++++++ .../Remover/AdjustmentsRemoverInterface.php | 25 ++++++++ .../OrderProcessing/OrderRecalculatorSpec.php | 14 ++++- .../spec/Remover/AdjustmentsRemoverSpec.php | 58 +++++++++++++++++++ 6 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 src/Sylius/Component/Core/Remover/AdjustmentsRemover.php create mode 100644 src/Sylius/Component/Core/Remover/AdjustmentsRemoverInterface.php create mode 100644 src/Sylius/Component/Core/spec/Remover/AdjustmentsRemoverSpec.php diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml index 31a464fa760..9f9c0d03dde 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml @@ -40,6 +40,8 @@ Sylius\Component\Core\OrderProcessing\PricesRecalculator Sylius\Component\Core\OrderProcessing\OrderRecalculator + Sylius\Component\Core\Remover\AdjustmentsRemover + Sylius\Component\Core\Taxation\Applicator\OrderShipmentTaxesApplicator Sylius\Component\Core\Taxation\Applicator\OrderItemsTaxesApplicator Sylius\Component\Core\Taxation\Applicator\OrderItemUnitsTaxesApplicator @@ -314,6 +316,7 @@ + @@ -332,6 +335,8 @@ + + diff --git a/src/Sylius/Component/Core/OrderProcessing/OrderRecalculator.php b/src/Sylius/Component/Core/OrderProcessing/OrderRecalculator.php index 969b5140c7c..572b9ebe50f 100644 --- a/src/Sylius/Component/Core/OrderProcessing/OrderRecalculator.php +++ b/src/Sylius/Component/Core/OrderProcessing/OrderRecalculator.php @@ -12,6 +12,7 @@ namespace Sylius\Component\Core\OrderProcessing; use Sylius\Component\Core\Model\OrderInterface; +use Sylius\Component\Core\Remover\AdjustmentsRemoverInterface; use Sylius\Component\Core\Taxation\Processor\OrderTaxesProcessorInterface; use Sylius\Component\Promotion\Processor\PromotionProcessorInterface; @@ -20,6 +21,11 @@ */ class OrderRecalculator implements OrderRecalculatorInterface { + /** + * @var AdjustmentsRemoverInterface + */ + private $adjustmentsRemover; + /** * @var OrderTaxesProcessorInterface */ @@ -47,11 +53,13 @@ class OrderRecalculator implements OrderRecalculatorInterface * @param ShippingChargesProcessorInterface $shippingChargesProcessor */ public function __construct( + AdjustmentsRemoverInterface $adjustmentsRemover, OrderTaxesProcessorInterface $taxesProcessor, PricesRecalculatorInterface $pricesRecalculator, PromotionProcessorInterface $promotionProcessor, ShippingChargesProcessorInterface $shippingChargesProcessor ) { + $this->adjustmentsRemover = $adjustmentsRemover; $this->taxesProcessor = $taxesProcessor; $this->pricesRecalculator = $pricesRecalculator; $this->promotionProcessor = $promotionProcessor; @@ -65,6 +73,7 @@ public function __construct( */ public function recalculate(OrderInterface $order) { + $this->adjustmentsRemover->remove($order); $this->pricesRecalculator->recalculate($order); $this->shippingChargesProcessor->applyShippingCharges($order); $this->promotionProcessor->process($order); diff --git a/src/Sylius/Component/Core/Remover/AdjustmentsRemover.php b/src/Sylius/Component/Core/Remover/AdjustmentsRemover.php new file mode 100644 index 00000000000..d1a3154c299 --- /dev/null +++ b/src/Sylius/Component/Core/Remover/AdjustmentsRemover.php @@ -0,0 +1,55 @@ + + */ +class AdjustmentsRemover implements AdjustmentsRemoverInterface +{ + /** + * @var array + */ + protected $adjustmentsToRemove = [ + AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT, + AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT, + AdjustmentInterface::TAX_ADJUSTMENT + ]; + + /** + * {@inheritdoc} + */ + public function remove(OrderInterface $order) + { + $units = $order->getItemUnits(); + foreach ($this->adjustmentsToRemove as $type) { + $order->removeAdjustments($type); + $this->removeUnitsAdjustments($units, $type); + } + } + + /** + * @param \Traversable $units + * @param string $type + */ + private function removeUnitsAdjustments(\Traversable $units, $type) + { + /** @var OrderItemUnitInterface $unit */ + foreach ($units as $unit) { + $unit->removeAdjustments($type); + } + } +} diff --git a/src/Sylius/Component/Core/Remover/AdjustmentsRemoverInterface.php b/src/Sylius/Component/Core/Remover/AdjustmentsRemoverInterface.php new file mode 100644 index 00000000000..f3f91c0306a --- /dev/null +++ b/src/Sylius/Component/Core/Remover/AdjustmentsRemoverInterface.php @@ -0,0 +1,25 @@ + + */ +interface AdjustmentsRemoverInterface +{ + /** + * @param OrderInterface $order + */ + public function remove(OrderInterface $order); +} diff --git a/src/Sylius/Component/Core/spec/OrderProcessing/OrderRecalculatorSpec.php b/src/Sylius/Component/Core/spec/OrderProcessing/OrderRecalculatorSpec.php index 1c2d6b89b33..8c1b5dc0eef 100644 --- a/src/Sylius/Component/Core/spec/OrderProcessing/OrderRecalculatorSpec.php +++ b/src/Sylius/Component/Core/spec/OrderProcessing/OrderRecalculatorSpec.php @@ -17,6 +17,7 @@ use Sylius\Component\Core\OrderProcessing\OrderRecalculatorInterface; use Sylius\Component\Core\OrderProcessing\PricesRecalculatorInterface; use Sylius\Component\Core\OrderProcessing\ShippingChargesProcessorInterface; +use Sylius\Component\Core\Remover\AdjustmentsRemoverInterface; use Sylius\Component\Core\Taxation\Processor\OrderTaxesProcessorInterface; use Sylius\Component\Promotion\Processor\PromotionProcessorInterface; @@ -28,12 +29,19 @@ class OrderRecalculatorSpec extends ObjectBehavior { function let( + AdjustmentsRemoverInterface $adjustmentsRemover, OrderTaxesProcessorInterface $taxesProcessor, PricesRecalculatorInterface $pricesRecalculator, PromotionProcessorInterface $promotionProcessor, ShippingChargesProcessorInterface $shippingChargesProcessor ) { - $this->beConstructedWith($taxesProcessor, $pricesRecalculator, $promotionProcessor, $shippingChargesProcessor); + $this->beConstructedWith( + $adjustmentsRemover, + $taxesProcessor, + $pricesRecalculator, + $promotionProcessor, + $shippingChargesProcessor + ); } function it_is_initializable() @@ -47,13 +55,15 @@ function it_implements_order_recalculator_interface() } function it_recalculates_order_promotions_taxes_and_shipping_charges( + AdjustmentsRemoverInterface $adjustmentsRemover, PromotionProcessorInterface $promotionProcessor, PricesRecalculatorInterface $pricesRecalculator, OrderTaxesProcessorInterface $taxesProcessor, ShippingChargesProcessorInterface $shippingChargesProcessor, OrderInterface $order ) { - $pricesRecalculator->recalculate($order); + $adjustmentsRemover->remove($order)->shouldBeCalled(); + $pricesRecalculator->recalculate($order)->shouldBeCalled(); $promotionProcessor->process($order)->shouldBeCalled(); $taxesProcessor->process($order)->shouldBeCalled(); $shippingChargesProcessor->applyShippingCharges($order)->shouldBeCalled(); diff --git a/src/Sylius/Component/Core/spec/Remover/AdjustmentsRemoverSpec.php b/src/Sylius/Component/Core/spec/Remover/AdjustmentsRemoverSpec.php new file mode 100644 index 00000000000..851b867ef87 --- /dev/null +++ b/src/Sylius/Component/Core/spec/Remover/AdjustmentsRemoverSpec.php @@ -0,0 +1,58 @@ + + */ +class AdjustmentsRemoverSpec extends ObjectBehavior +{ + function it_is_initializable() + { + $this->shouldHaveType('Sylius\Component\Core\Remover\AdjustmentsRemover'); + } + + function it_implements_adjustments_remover_interface() + { + $this->shouldImplement(AdjustmentsRemoverInterface::class); + } + + function it_removes_adjustments_from_order_recursively( + OrderInterface $order, + OrderItemUnitInterface $firstUnit, + OrderItemUnitInterface $secondUnit + ) { + $order->removeAdjustments(AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT)->shouldBeCalled(); + $order->removeAdjustments(AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT)->shouldBeCalled(); + $order->removeAdjustments(AdjustmentInterface::TAX_ADJUSTMENT)->shouldBeCalled(); + + $order->getItemUnits()->willReturn(new \ArrayIterator([$firstUnit->getWrappedObject(), $secondUnit->getWrappedObject()])); + + $firstUnit->removeAdjustments(AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT)->shouldBeCalled(); + $firstUnit->removeAdjustments(AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT)->shouldBeCalled(); + $firstUnit->removeAdjustments(AdjustmentInterface::TAX_ADJUSTMENT)->shouldBeCalled(); + $secondUnit->removeAdjustments(AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT)->shouldBeCalled(); + $secondUnit->removeAdjustments(AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT)->shouldBeCalled(); + $secondUnit->removeAdjustments(AdjustmentInterface::TAX_ADJUSTMENT)->shouldBeCalled(); + + $this->remove($order); + } +} From 08bc640223ac97a00a08dfa6a22c2791ef0dec17 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Fri, 20 May 2016 12:50:25 +0200 Subject: [PATCH 2/2] PR review fixes --- .../OrderProcessing/OrderRecalculator.php | 4 +- .../Core/Remover/AdjustmentsRemover.php | 33 ++++----------- .../Remover/AdjustmentsRemoverInterface.php | 2 +- .../OrderProcessing/OrderRecalculatorSpec.php | 2 +- .../spec/Remover/AdjustmentsRemoverSpec.php | 24 +++-------- src/Sylius/Component/Order/Model/Order.php | 11 +++++ .../Component/Order/Model/OrderInterface.php | 5 +++ .../Component/Order/spec/Model/OrderSpec.php | 42 +++++++++++++++++++ 8 files changed, 77 insertions(+), 46 deletions(-) diff --git a/src/Sylius/Component/Core/OrderProcessing/OrderRecalculator.php b/src/Sylius/Component/Core/OrderProcessing/OrderRecalculator.php index 572b9ebe50f..4bb782d7dde 100644 --- a/src/Sylius/Component/Core/OrderProcessing/OrderRecalculator.php +++ b/src/Sylius/Component/Core/OrderProcessing/OrderRecalculator.php @@ -18,6 +18,7 @@ /** * @author Jan Góralski + * @author Mateusz Zalewski */ class OrderRecalculator implements OrderRecalculatorInterface { @@ -47,6 +48,7 @@ class OrderRecalculator implements OrderRecalculatorInterface private $shippingChargesProcessor; /** + * @param AdjustmentsRemoverInterface $adjustmentsRemover * @param OrderTaxesProcessorInterface $taxesProcessor * @param PricesRecalculatorInterface $pricesRecalculator * @param PromotionProcessorInterface $promotionProcessor @@ -73,7 +75,7 @@ public function __construct( */ public function recalculate(OrderInterface $order) { - $this->adjustmentsRemover->remove($order); + $this->adjustmentsRemover->removeFrom($order); $this->pricesRecalculator->recalculate($order); $this->shippingChargesProcessor->applyShippingCharges($order); $this->promotionProcessor->process($order); diff --git a/src/Sylius/Component/Core/Remover/AdjustmentsRemover.php b/src/Sylius/Component/Core/Remover/AdjustmentsRemover.php index d1a3154c299..a4f9a33ba52 100644 --- a/src/Sylius/Component/Core/Remover/AdjustmentsRemover.php +++ b/src/Sylius/Component/Core/Remover/AdjustmentsRemover.php @@ -20,36 +20,19 @@ */ class AdjustmentsRemover implements AdjustmentsRemoverInterface { - /** - * @var array - */ - protected $adjustmentsToRemove = [ - AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT, - AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT, - AdjustmentInterface::TAX_ADJUSTMENT - ]; - /** * {@inheritdoc} */ - public function remove(OrderInterface $order) + public function removeFrom(OrderInterface $order) { - $units = $order->getItemUnits(); - foreach ($this->adjustmentsToRemove as $type) { - $order->removeAdjustments($type); - $this->removeUnitsAdjustments($units, $type); - } - } + $adjustmentsToRemove = [ + AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT, + AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT, + AdjustmentInterface::TAX_ADJUSTMENT + ]; - /** - * @param \Traversable $units - * @param string $type - */ - private function removeUnitsAdjustments(\Traversable $units, $type) - { - /** @var OrderItemUnitInterface $unit */ - foreach ($units as $unit) { - $unit->removeAdjustments($type); + foreach ($adjustmentsToRemove as $type) { + $order->removeAdjustmentsRecursively($type); } } } diff --git a/src/Sylius/Component/Core/Remover/AdjustmentsRemoverInterface.php b/src/Sylius/Component/Core/Remover/AdjustmentsRemoverInterface.php index f3f91c0306a..4ed259a1244 100644 --- a/src/Sylius/Component/Core/Remover/AdjustmentsRemoverInterface.php +++ b/src/Sylius/Component/Core/Remover/AdjustmentsRemoverInterface.php @@ -21,5 +21,5 @@ interface AdjustmentsRemoverInterface /** * @param OrderInterface $order */ - public function remove(OrderInterface $order); + public function removeFrom(OrderInterface $order); } diff --git a/src/Sylius/Component/Core/spec/OrderProcessing/OrderRecalculatorSpec.php b/src/Sylius/Component/Core/spec/OrderProcessing/OrderRecalculatorSpec.php index 8c1b5dc0eef..b95b323ba78 100644 --- a/src/Sylius/Component/Core/spec/OrderProcessing/OrderRecalculatorSpec.php +++ b/src/Sylius/Component/Core/spec/OrderProcessing/OrderRecalculatorSpec.php @@ -62,7 +62,7 @@ function it_recalculates_order_promotions_taxes_and_shipping_charges( ShippingChargesProcessorInterface $shippingChargesProcessor, OrderInterface $order ) { - $adjustmentsRemover->remove($order)->shouldBeCalled(); + $adjustmentsRemover->removeFrom($order)->shouldBeCalled(); $pricesRecalculator->recalculate($order)->shouldBeCalled(); $promotionProcessor->process($order)->shouldBeCalled(); $taxesProcessor->process($order)->shouldBeCalled(); diff --git a/src/Sylius/Component/Core/spec/Remover/AdjustmentsRemoverSpec.php b/src/Sylius/Component/Core/spec/Remover/AdjustmentsRemoverSpec.php index 851b867ef87..a522c5872ec 100644 --- a/src/Sylius/Component/Core/spec/Remover/AdjustmentsRemoverSpec.php +++ b/src/Sylius/Component/Core/spec/Remover/AdjustmentsRemoverSpec.php @@ -35,24 +35,12 @@ function it_implements_adjustments_remover_interface() $this->shouldImplement(AdjustmentsRemoverInterface::class); } - function it_removes_adjustments_from_order_recursively( - OrderInterface $order, - OrderItemUnitInterface $firstUnit, - OrderItemUnitInterface $secondUnit - ) { - $order->removeAdjustments(AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT)->shouldBeCalled(); - $order->removeAdjustments(AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT)->shouldBeCalled(); - $order->removeAdjustments(AdjustmentInterface::TAX_ADJUSTMENT)->shouldBeCalled(); - - $order->getItemUnits()->willReturn(new \ArrayIterator([$firstUnit->getWrappedObject(), $secondUnit->getWrappedObject()])); - - $firstUnit->removeAdjustments(AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT)->shouldBeCalled(); - $firstUnit->removeAdjustments(AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT)->shouldBeCalled(); - $firstUnit->removeAdjustments(AdjustmentInterface::TAX_ADJUSTMENT)->shouldBeCalled(); - $secondUnit->removeAdjustments(AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT)->shouldBeCalled(); - $secondUnit->removeAdjustments(AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT)->shouldBeCalled(); - $secondUnit->removeAdjustments(AdjustmentInterface::TAX_ADJUSTMENT)->shouldBeCalled(); + function it_removes_adjustments_from_order_recursively(OrderInterface $order) + { + $order->removeAdjustmentsRecursively(AdjustmentInterface::ORDER_PROMOTION_ADJUSTMENT)->shouldBeCalled(); + $order->removeAdjustmentsRecursively(AdjustmentInterface::ORDER_ITEM_PROMOTION_ADJUSTMENT)->shouldBeCalled(); + $order->removeAdjustmentsRecursively(AdjustmentInterface::TAX_ADJUSTMENT)->shouldBeCalled(); - $this->remove($order); + $this->removeFrom($order); } } diff --git a/src/Sylius/Component/Order/Model/Order.php b/src/Sylius/Component/Order/Model/Order.php index aed4d2a2d6a..53d4851529f 100644 --- a/src/Sylius/Component/Order/Model/Order.php +++ b/src/Sylius/Component/Order/Model/Order.php @@ -460,6 +460,17 @@ public function removeAdjustments($type) } } + /** + * {@inheritdoc} + */ + public function removeAdjustmentsRecursively($type = null) + { + $this->removeAdjustments($type); + foreach ($this->items as $item) { + $item->removeAdjustmentsRecursively($type); + } + } + /** * {@inheritdoc} */ diff --git a/src/Sylius/Component/Order/Model/OrderInterface.php b/src/Sylius/Component/Order/Model/OrderInterface.php index 5b117d94387..62246a5f92f 100644 --- a/src/Sylius/Component/Order/Model/OrderInterface.php +++ b/src/Sylius/Component/Order/Model/OrderInterface.php @@ -158,4 +158,9 @@ public function getAdjustmentsRecursively($type = null); * @return int */ public function getAdjustmentsTotalRecursively($type = null); + + /** + * @param string|null $type + */ + public function removeAdjustmentsRecursively($type = null); } diff --git a/src/Sylius/Component/Order/spec/Model/OrderSpec.php b/src/Sylius/Component/Order/spec/Model/OrderSpec.php index 1a7f4a7591f..1b4bd187b7a 100644 --- a/src/Sylius/Component/Order/spec/Model/OrderSpec.php +++ b/src/Sylius/Component/Order/spec/Model/OrderSpec.php @@ -201,6 +201,48 @@ function it_removes_adjustments_properly(AdjustmentInterface $adjustment) $this->hasAdjustment($adjustment)->shouldReturn(false); } + function it_removes_adjustments_recursively_properly( + AdjustmentInterface $orderAdjustment, + OrderItemInterface $item + ) { + $this->addAdjustment($orderAdjustment); + $this->addItem($item); + + $item->removeAdjustmentsRecursively(null)->shouldBeCalled(); + + $this->removeAdjustmentsRecursively(); + + $this->hasAdjustment($orderAdjustment)->shouldReturn(false); + } + + function it_removes_adjustments_recursively_by_type_properly( + AdjustmentInterface $orderPromotionAdjustment, + AdjustmentInterface $orderTaxAdjustment, + OrderItemInterface $item + ) { + $orderPromotionAdjustment->getType()->willReturn('promotion'); + $orderPromotionAdjustment->isNeutral()->willReturn(true); + $orderPromotionAdjustment->setAdjustable($this)->shouldBeCalled(); + $orderPromotionAdjustment->isLocked()->willReturn(false); + + $orderTaxAdjustment->getType()->willReturn('tax'); + $orderTaxAdjustment->isNeutral()->willReturn(true); + $orderTaxAdjustment->setAdjustable($this)->shouldBeCalled(); + $orderTaxAdjustment->isLocked()->willReturn(false); + + $this->addAdjustment($orderPromotionAdjustment); + $this->addAdjustment($orderTaxAdjustment); + $this->addItem($item); + + $item->removeAdjustmentsRecursively('tax')->shouldBeCalled(); + $orderTaxAdjustment->setAdjustable(null)->shouldBeCalled(); + + $this->removeAdjustmentsRecursively('tax'); + + $this->hasAdjustment($orderPromotionAdjustment)->shouldReturn(true); + $this->hasAdjustment($orderTaxAdjustment)->shouldReturn(false); + } + function it_returns_adjustments_recursively( AdjustmentInterface $orderAdjustment, AdjustmentInterface $itemAdjustment1,