Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Price recalculation #3845

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions app/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1295,7 +1295,10 @@ services:
arguments:
$fleetKey: '%tile38_fleet_key%'

AppBundle\Pricing\PricingManager: ~

AppBundle\Pricing\PricingManager:
arguments:
$eventBus: '@event_bus'

AppBundle\Form\BusinessAccountRegistrationFlow:
autoconfigure: true
Expand Down Expand Up @@ -1329,4 +1332,4 @@ services:
arguments:
$pixabayApiKey: '%env(PIXABAY_API_KEY)%'

AppBundle\Utils\RestaurantDecorator: ~
AppBundle\Utils\RestaurantDecorator: ~
29 changes: 22 additions & 7 deletions src/Doctrine/EventSubscriber/TaskSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 AppBundle\Sylius\Order\OrderInterface;
Expand All @@ -19,7 +19,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;

Check failure on line 22 in src/Doctrine/EventSubscriber/TaskSubscriber.php

View workflow job for this annotation

GitHub Actions / Lint PHP (8.1)

Cannot use Sylius\Component\Order\Model\OrderInterface as OrderInterface because the name is already in use on line 22

class TaskSubscriber implements EventSubscriber
{
Expand All @@ -34,12 +34,13 @@
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;
Expand Down Expand Up @@ -239,5 +240,19 @@
}
}
}

// 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();
}
}
}

}
65 changes: 65 additions & 0 deletions src/Domain/Order/Event/OrderPriceRecalculated.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

namespace AppBundle\Domain\Order\Event;

use AppBundle\Domain\DomainEvent;
use AppBundle\Domain\HasIconInterface;
use AppBundle\Domain\Order\Event;
use AppBundle\Sylius\Order\OrderInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;

class OrderPriceRecalculated extends Event implements DomainEvent, HasIconInterface
{

public function __construct(
OrderInterface $order,
private int $new_price,
private int $old_price,
private ?string $caused_by = null
)
{
parent::__construct($order);
}

public function getNewPrice(): int
{
return $this->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';
}
}
6 changes: 3 additions & 3 deletions src/Entity/Delivery.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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()) {

Expand All @@ -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();
Expand Down Expand Up @@ -433,7 +434,6 @@ private static function createTaskObject(?Task $task)
{
$taskObject = new \stdClass();
if ($task) {

return $task->toExpressionLanguageObject();
}

Expand Down
11 changes: 11 additions & 0 deletions src/Entity/Sylius/Order.php
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,17 @@ public function hasPayments(): bool
return !$this->payments->isEmpty();
}

/**
* @return 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)
return PaymentInterface::STATE_COMPLETED === $payment->getState();
})->count() > 0;
}

/**
* {@inheritdoc}
*/
Expand Down
93 changes: 87 additions & 6 deletions src/Pricing/PricingManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,29 @@

namespace AppBundle\Pricing;

use ApiPlatform\Core\Api\IriConverterInterface;
use AppBundle\Domain\Order\Event\OrderPriceRecalculated;
use AppBundle\Entity\Delivery;
use AppBundle\Exception\Pricing\NoRuleMatchedException;
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
)
{}

/**
Expand Down Expand Up @@ -63,4 +67,81 @@ public function createOrder(Delivery $delivery, bool $throwException = false): ?

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->isPaid()) {
$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;
}
}
19 changes: 16 additions & 3 deletions src/Service/DeliveryManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {

Expand Down Expand Up @@ -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);
}
}
1 change: 1 addition & 0 deletions src/Utils/OrderEventCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class OrderEventCollection extends ArrayCollection
'order:picked',
'order:dropped',
'order:cancelled',
'order:price_recalculated'
];

public function __construct(OrderInterface $order)
Expand Down
Loading