From d69b0184baf356a472fbaf97785d416ebe80068d Mon Sep 17 00:00:00 2001 From: Antoine Bagnaud Date: Tue, 21 Nov 2023 11:26:44 +0100 Subject: [PATCH 1/2] feat(order): update price when task is cancelled --- app/config/services.yml | 4 +- .../EventSubscriber/TaskSubscriber.php | 29 ++++-- .../Order/Event/OrderPriceRecalculated.php | 65 +++++++++++++ src/Entity/Delivery.php | 6 +- src/Entity/Sylius/Order.php | 11 +++ src/Pricing/PricingManager.php | 93 +++++++++++++++++-- src/Service/DeliveryManager.php | 19 +++- src/Utils/OrderEventCollection.php | 1 + 8 files changed, 208 insertions(+), 20 deletions(-) create mode 100644 src/Domain/Order/Event/OrderPriceRecalculated.php diff --git a/app/config/services.yml b/app/config/services.yml index 52d93a010c..e9c6513be3 100755 --- a/app/config/services.yml +++ b/app/config/services.yml @@ -1278,4 +1278,6 @@ services: arguments: $fleetKey: '%tile38_fleet_key%' - AppBundle\Pricing\PricingManager: ~ + AppBundle\Pricing\PricingManager: + arguments: + $eventBus: '@event_bus' diff --git a/src/Doctrine/EventSubscriber/TaskSubscriber.php b/src/Doctrine/EventSubscriber/TaskSubscriber.php index 2a11e37f8b..da0a12d567 100644 --- a/src/Doctrine/EventSubscriber/TaskSubscriber.php +++ b/src/Doctrine/EventSubscriber/TaskSubscriber.php @@ -6,8 +6,8 @@ use AppBundle\Domain\EventStore; use AppBundle\Domain\Task\Event\TaskCreated; use AppBundle\Entity\Address; -use AppBundle\Entity\Delivery; use AppBundle\Entity\Task; +use AppBundle\Pricing\PricingManager; use AppBundle\Service\Geocoder; use AppBundle\Service\OrderManager; use Doctrine\Common\EventSubscriber; @@ -18,7 +18,7 @@ use Doctrine\ORM\UnitOfWork; use Psr\Log\LoggerInterface; use SimpleBus\Message\Bus\MessageBus; -use Symfony\Component\Messenger\MessageBusInterface; +use Sylius\Component\Order\Model\OrderInterface; class TaskSubscriber implements EventSubscriber { @@ -33,12 +33,13 @@ class TaskSubscriber implements EventSubscriber private $createdAddresses; public function __construct( - MessageBus $eventBus, - EventStore $eventStore, + MessageBus $eventBus, + EventStore $eventStore, EntityChangeSetProcessor $processor, - LoggerInterface $logger, - Geocoder $geocoder, - private OrderManager $orderManager + LoggerInterface $logger, + Geocoder $geocoder, + private OrderManager $orderManager, + private PricingManager $pricingManager ) { $this->eventBus = $eventBus; @@ -222,5 +223,19 @@ private function handleStateChangesForTasks(EntitymanagerInterface $em, array $t } } } + + // Update pricing in a different loop to avoid race condition on cancelled orders + foreach ($tasksToUpdate as $taskToUpdate) { + $delivery = $taskToUpdate->getDelivery(); + if ( + $delivery !== null && + ($order = $delivery->getOrder()) !== null && + $order->getState() !== OrderInterface::STATE_CANCELLED + ) { + $this->pricingManager->updateOrder($delivery, $taskToUpdate); + $em->flush(); + } + } } + } diff --git a/src/Domain/Order/Event/OrderPriceRecalculated.php b/src/Domain/Order/Event/OrderPriceRecalculated.php new file mode 100644 index 0000000000..b5d7850483 --- /dev/null +++ b/src/Domain/Order/Event/OrderPriceRecalculated.php @@ -0,0 +1,65 @@ +new_price; + } + + public function getOldPrice(): int + { + return $this->old_price; + } + + public function getCausedby(): ?string + { + return $this->caused_by; + } + + public function toPayload() + { + return [ + 'price' => $this->getNewPrice(), + 'old_price' => $this->getOldPrice(), + 'caused_by' => $this->getCausedby() + ]; + } + + public function normalize(NormalizerInterface $serializer) + { + return array_merge( + parent::normalize($serializer), + ['caused_by' => $this->getCausedby()] + ); + } + + public static function iconName() + { + return 'calculator'; + } + + public static function messageName(): string + { + return 'order:price_recalculated'; + } +} diff --git a/src/Entity/Delivery.php b/src/Entity/Delivery.php index 4f86c46217..4f3a2c1c38 100644 --- a/src/Entity/Delivery.php +++ b/src/Entity/Delivery.php @@ -187,7 +187,7 @@ public function setOrder(OrderInterface $order) public function getWeight() { $totalWeight = null; - foreach ($this->getTasks() as $task) { + foreach ($this->getTasks('not task.isCancelled()') as $task) { if (null !== $task->getWeight()) { $totalWeight += $task->getWeight(); } @@ -372,6 +372,7 @@ public function isAssigned() public function isCompleted() { + //TODO: Should we check if all tasks are completed or only active ones? foreach ($this->getTasks() as $task) { if (!$task->isCompleted()) { @@ -388,7 +389,7 @@ public function getPackages() $hash = new \SplObjectStorage(); - foreach ($this->getTasks() as $task) { + foreach ($this->getTasks('not task.isCancelled()') as $task) { if ($task->hasPackages()) { foreach ($task->getPackages() as $package) { $object = $package->getPackage(); @@ -433,7 +434,6 @@ private static function createTaskObject(?Task $task) { $taskObject = new \stdClass(); if ($task) { - return $task->toExpressionLanguageObject(); } diff --git a/src/Entity/Sylius/Order.php b/src/Entity/Sylius/Order.php index ed28bb2870..b476b021fd 100644 --- a/src/Entity/Sylius/Order.php +++ b/src/Entity/Sylius/Order.php @@ -740,6 +740,17 @@ public function hasPayments(): bool return !$this->payments->isEmpty(); } + /** + * @return bool + */ + public function isPayed(): bool + { + return $this->getPayments()->filter(function (PaymentInterface $payment) { + //TODO: Check if payment is completed only in this state (e.g. PaymentInterface::STATE_PROCESSING) + return PaymentInterface::STATE_COMPLETED === $payment->getState(); + })->count() > 0; + } + /** * {@inheritdoc} */ diff --git a/src/Pricing/PricingManager.php b/src/Pricing/PricingManager.php index c5f649d1ed..23f647ea12 100644 --- a/src/Pricing/PricingManager.php +++ b/src/Pricing/PricingManager.php @@ -2,24 +2,28 @@ namespace AppBundle\Pricing; +use ApiPlatform\Core\Api\IriConverterInterface; +use AppBundle\Domain\Order\Event\OrderPriceRecalculated; use AppBundle\Entity\Delivery; use AppBundle\Service\DeliveryManager; use AppBundle\Service\OrderManager; -use AppBundle\Sylius\Customer\CustomerInterface; use AppBundle\Sylius\Order\OrderFactory; use AppBundle\Sylius\Order\OrderInterface; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; -use Symfony\Component\HttpFoundation\Request; +use SimpleBus\Message\Bus\MessageBus; class PricingManager { public function __construct( - private DeliveryManager $deliveryManager, - private OrderManager $orderManager, - private OrderFactory $orderFactory, + private DeliveryManager $deliveryManager, + private OrderManager $orderManager, + private OrderFactory $orderFactory, private EntityManagerInterface $entityManager, - private LoggerInterface $logger) + private LoggerInterface $logger, + private MessageBus $eventBus, + private IriConverterInterface $iriConverter + ) {} /** @@ -57,4 +61,81 @@ public function createOrder(Delivery $delivery): ?OrderInterface return null; } + + public function updateOrder(Delivery $delivery, ?object $triggered_by = null): ?OrderInterface + { + /** @var ?OrderInterface $order */ + $order = $delivery->getOrder(); + if (is_null($order)) { + $this->logger->info("No order set to this delivery, skipping"); + return null; + } + + if ($order->isPayed()) { + $this->logger->info("Order is already payed, skipping"); + return null; + } + + $store = $delivery->getStore(); + if (is_null($store)) { + $this->logger->info("No store set to this delivery, skipping"); + return null; + } + + if ($order->getItems()->count() > 1) { + $this->logger->info("Order has more than one item, skipping"); + return null; + } + + $delivery->setDistance(ceil($this->deliveryManager->calculateDistance( + $delivery, + $delivery->getTasks('not task.isCancelled()') + ))); + + if ($order->getItems()->first()->isImmutable()) { + $this->logger->info("Order item is immutable, skipping"); + return null; + } + + $old_price = $order->getItems()->first()->getUnitPrice(); + + $price = $this->deliveryManager->getPrice($delivery, $store->getPricingRuleSet()); + if (is_null($price)) { + $this->logger->error('Price could not be calculated'); + return null; + } + + // Early exit if price didn't change + if ($old_price === $price) { + $this->logger->info("Price didn't change, skipping"); + return $order; + } + + $order->getItems()->map(function ($item) use ($price) { + $item->setUnitPrice((int)$price); + }); + + $order->recalculateItemsTotal(); + $order->recalculateAdjustmentsTotal(); + + // If everything is fine, remove the payment + // TODO: See if other behavior is needed + // TODO: Move payments logic changes to a listener of OrderPriceRecalculated + $order->getPayments()->map(function ($payment) use (&$order) { + $order->removePayment($payment); + }); + + $this->entityManager->persist($order); + + $this->eventBus->handle(new OrderPriceRecalculated( + $order, + $price, + $old_price, + $this->iriConverter->getIriFromItem($triggered_by), + ) + ); + + $this->entityManager->flush(); + return $order; + } } diff --git a/src/Service/DeliveryManager.php b/src/Service/DeliveryManager.php index 0b01b131a8..b07a44f1a2 100644 --- a/src/Service/DeliveryManager.php +++ b/src/Service/DeliveryManager.php @@ -65,7 +65,7 @@ public function getPrice(Delivery $delivery, PricingRuleSet $ruleSet) $matchedAtLeastOne = false; if (count($delivery->getTasks()) > 2 || $ruleSet->hasOption(PricingRuleSet::OPTION_MAP_ALL_TASKS)) { - foreach ($delivery->getTasks() as $task) { + foreach ($delivery->getTasks('not task.isCancelled()') as $task) { foreach ($ruleSet->getRules() as $rule) { if ($task->matchesPricingRule($rule, $this->expressionLanguage)) { @@ -190,9 +190,22 @@ public function setDefaults(Delivery $delivery) } } - $coords = array_map(fn ($task) => $task->getAddress()->getGeo(), $delivery->getTasks()); - $distance = $this->routing->getDistance(...$coords); + $distance = $this->calculateDistance($delivery); $delivery->setDistance(ceil($distance)); } + + /** + * @param Delivery $delivery + * @param ?Task[] $tasks + * @return float + */ + public function calculateDistance(Delivery $delivery, ?array $tasks = null): float + { + if (is_null($tasks)) { + $tasks = $delivery->getTasks(); + } + $coords = array_map(fn($task) => $task->getAddress()->getGeo(), $tasks); + return $this->routing->getDistance(...$coords); + } } diff --git a/src/Utils/OrderEventCollection.php b/src/Utils/OrderEventCollection.php index 40951cb5f7..5ebd0ea654 100644 --- a/src/Utils/OrderEventCollection.php +++ b/src/Utils/OrderEventCollection.php @@ -14,6 +14,7 @@ class OrderEventCollection extends ArrayCollection 'order:picked', 'order:dropped', 'order:cancelled', + 'order:price_recalculated' ]; public function __construct(OrderInterface $order) From 57ab3c0a58c8af49f0c2dbe081e6fecb8407926d Mon Sep 17 00:00:00 2001 From: Antoine Bagnaud Date: Mon, 8 Jan 2024 14:23:01 +0100 Subject: [PATCH 2/2] fix typo. --- src/Entity/Sylius/Order.php | 2 +- src/Pricing/PricingManager.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Entity/Sylius/Order.php b/src/Entity/Sylius/Order.php index b476b021fd..e614aceece 100644 --- a/src/Entity/Sylius/Order.php +++ b/src/Entity/Sylius/Order.php @@ -743,7 +743,7 @@ public function hasPayments(): bool /** * @return bool */ - public function isPayed(): bool + public function isPaid(): bool { return $this->getPayments()->filter(function (PaymentInterface $payment) { //TODO: Check if payment is completed only in this state (e.g. PaymentInterface::STATE_PROCESSING) diff --git a/src/Pricing/PricingManager.php b/src/Pricing/PricingManager.php index 23f647ea12..b85fcfcbac 100644 --- a/src/Pricing/PricingManager.php +++ b/src/Pricing/PricingManager.php @@ -71,7 +71,7 @@ public function updateOrder(Delivery $delivery, ?object $triggered_by = null): ? return null; } - if ($order->isPayed()) { + if ($order->isPaid()) { $this->logger->info("Order is already payed, skipping"); return null; }