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

[BUG] Falsy order of Events in Cart/OrderController #571

Open
rintisch opened this issue Sep 30, 2024 · 4 comments
Open

[BUG] Falsy order of Events in Cart/OrderController #571

rintisch opened this issue Sep 30, 2024 · 4 comments
Assignees
Labels
9.x related to TYPO3 v12 bug

Comments

@rintisch
Copy link
Collaborator

Bug Report

Current Behavior
According to the docs https://docs.typo3.org/p/extcode/cart/main/en-us/Developer/Events/Index.html#confval-extcode-cart-event-order-paymentevent the PaymentEvent should trigger the StockEvent on success. But in the code the StockEvent comes after the PaymentEvent:

$stockEvent = new StockEvent($this->cart, $orderItem, $this->configurations);
$this->eventDispatcher->dispatch($stockEvent);
if ($stockEvent instanceof StoppableEventInterface && $stockEvent->isPropagationStopped()) {
return true;
}
$paymentEvent = new PaymentEvent($this->cart, $orderItem, $this->configurations);
$this->eventDispatcher->dispatch($paymentEvent);
if ($paymentEvent instanceof StoppableEventInterface && $paymentEvent->isPropagationStopped()) {
return true;
}

Due to this it can happen that the stock is changed although the purchase failed.

Expected behavior/output
Those two Events have to switch position.

Environment

  • cart version: 9.0
@rintisch rintisch added bug 9.x related to TYPO3 v12 labels Sep 30, 2024
@rintisch rintisch self-assigned this Sep 30, 2024
rintisch added a commit to rintisch/cart that referenced this issue Oct 1, 2024
The PaymentEvent has to be placed before the
StockEvent because the order can fail in the
PaymentEvent so the StockEvent has to be placed
after the PaymentEvent.
In case a PaymentProvider uses forwarding the
PaymentEvent and FinishEvent has to be triggered
within the EventListener of the PaymentEvent.
This is already described in the docs.

Fixes: extcode#571
@swisschocolate
Copy link

I've just stumbled across this behavior. If a order is submitted, the stock is changed immediatly, no matter if and how the external payment provider is processed. This leads to discrepancies in the inventory.

@extcode extcode self-assigned this Oct 13, 2024
@rintisch
Copy link
Collaborator Author

Yeah but I think extcode's arguments are reasonable:
#572 (comment)

You should adapt your payment provider. Or do you have other plans in the meanwhile @extcode ?

@extcode
Copy link
Owner

extcode commented Oct 14, 2024

There are various ways to solve the problem.

  • The payment providers ensure that the products already withdrawn from the warehouse management system are returned to the warehouse if an error occurs. That would still be my preferred method.
  • The payment providers would be changed so that payments are first released and then the order is only really finalized with a second click in the shopping cart and the payment is then also confirmed with the payment provider. In my view, this is a major undertaking.
  • In addition to the stock, a further value is also saved, which reserves the products that are currently in the order and then both the stock handling reacts to this, but also the payment provider removes the products from the reservation and finally books them out of the warehouse. This seems to me to be very similar to the first approach, because here too the payment provider has to remove products from the stock. However, it must also ensure that the reservation is tidied up and that only those products that play a role in the current order are deleted from the reservation (number reduced).

I would not swap the order, because otherwise customer A and customer B could buy the same product in competition because the payment process for one of them has not yet been completed. For stores in which the cart extension is used, this is probably an edge case and probably occurs rarely or not at all.
For your own implementation, you could possibly also remove one EventListener from one event and insert it again in a later event.
With cart-products 5.x for TYPO3 v12, however, this is probably no longer so easy because an explicit event is passed here and no longer the interface. The only option is to remove one EventListener and re-insert your own (copy and adapt) in the later event. There is also a “solution” with composer-patches to avoid the custom EventListener.

I will have a look at this for a payment provider in the near future.

@rintisch
Copy link
Collaborator Author

I just realized the following:

protected bool $wasOrdered = false;

The Extcode\Cart\Domain\Model\Cart (which is at the moment not stored by default but can be stored by a paymentProvider) has a property wasOrdered which is false by default. Can't this be used by a paymentProvider extension?

  • Before redirecting to the paymentProvider the cart must be stored (e.g. EXT:cart_paypal does that)
  • EXT:cart implements a Symfony command which must be triggered by a cron job. This command looks whether order which are older than a certain time exists which are marked as not ordered.
  • The command loops through the order items, looks whether they have stock handling and readds the amount of items which not were ordered.

That sounds not that difficult?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9.x related to TYPO3 v12 bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants