Magento PayPal checkout fails if email sending fails / other payment does not #13133
Conversation
| @@ -809,7 +809,11 @@ public function place($token, $shippingMethodCode = null) | |||
| case \Magento\Sales\Model\Order::STATE_PROCESSING: | |||
| case \Magento\Sales\Model\Order::STATE_COMPLETE: | |||
| case \Magento\Sales\Model\Order::STATE_PAYMENT_REVIEW: | |||
| $this->orderSender->send($order); | |||
| try { | |||
| $this->orderSender->send($order); | |||
There was a problem hiding this comment.
Code changes look good to me 👍
Please cover new logic with unit test, make an orderSender mock so that send is throwing exception and then assert exception was logged.
There was a problem hiding this comment.
@orlangur I tried to start doing this but it looks like it is not as simple as I thought. No tests exist for the placeOrder at this time so I'd need a quote management mock, an order mock as it returns an order, and then the order sender mock, and I'm not willing to commit so much time to learn all that and learn how to run individual tests.
If you can provide an existing test for placeOrder I'd likely be able to commit the time to build on it. But at the moment it's not something I'd be willing to proceed with further.
|
|
|
Hi @driskell , please, sign CLA, otherwise, we can't process your pull request |
|
Done! |
|
Thanks! |
… other payment does not #13133
|
Hi @driskell. Thank you for your contribution. Please, consider to port this solution to 2.3 release line. |
Description
If PayPal checkout is used and, for whatever reason, the email sender throws an exception (maybe third-party integration fails to send), the PayPal order is still placed but the user is shown Something went wrong message and ends up in review page.
This is in contrast to a cash payment or other payment method where the order is placed successfully.
This is because other payment methods run through this checkout, that wraps orderSender in a try/catch:
https://github.com/magento/magento2/blob/2.2-develop/app/code/Magento/Checkout/Model/Type/Onepage.php#L731
This PR replicates that try/catch for PayPal.
Manual testing scenarios
Contribution checklist