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

[BUGFIX] Switch order of PaymentEvent & StockEvent #572

Closed

Conversation

rintisch
Copy link
Collaborator

@rintisch rintisch commented 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: #571

@rintisch rintisch requested a review from extcode October 1, 2024 08:28
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
@rintisch rintisch force-pushed the bugfix/571-switch-order-of-events branch from baad7ce to c07b808 Compare October 1, 2024 08:29
@extcode
Copy link
Owner

extcode commented Oct 6, 2024

I think this change would already be a breaking change.

The original idea to choose this order was that a payment process can take a while until all the data has been entered. In the meantime, another user could buy the last available products and pay faster than the first user. A rather unlikely case, but still possible.
There are two possible solutions to this idea. Either the payment providers increase the stock again in the event of an error, or the extensions for the availability check include a storage system in which products and their number are saved during the payment process. In the event of incorrect payments, this storage is prepared accordingly and, if successful, the number is finally reduced in the product.

Since the current behavior of the payment providers is such that the order is saved even before the redirect to the payment provider and the order is marked as canceled in the event of an error, the stock should rather be updated again in the event of an error.
I'll have a think and see what needs to be adjusted in the product extensions.

I have to say that I have no idea how often the stock management in cart-products is really used.

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

rintisch commented Oct 7, 2024

That sounds reasonable!

I guess you mean you look what needs adaption in the payment extensions (instead of "product extensions")?

When no realizing this ticket we need to adapt the documentation: https://docs.typo3.org/p/extcode/cart/main/en-us/Developer/Events/Index.html#confval-extcode-cart-event-order-stockevent
→ It's named as "fourth event" (after the PaymentEvent) and that it needs to be triggered by the payment extension.

@swisschocolate
Copy link

All reasonable arguments, @extcode . Problem of current behavior is: if the payment fails – or even if the customer simply does not process the payment process – the stock remains too low. Therefore it's not clever if the payment provider rolls back the stock in case of a failed payment, because it has no chance to do so if the customer simply closes the payment window.
Better solution would be a configuration in the payment settings, whether the stock handling shall be processed by cart (current and standard behavior) or not. In case of latter the stock handling (and cart-emptying) is in responsibility of the payment provider and will then only processed if payment was successful. And yes, there is a risk of overbuying. But to be honest: this would be rare, at least rarer than a failed or aborted payment.

@rintisch
Copy link
Collaborator Author

I close this ticket due to the discussion in #571. Another solution will follow.

@rintisch rintisch closed this Oct 16, 2024
@rintisch rintisch deleted the bugfix/571-switch-order-of-events branch October 16, 2024 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Falsy order of Events in Cart/OrderController
3 participants