- Issue created by @GuillaumePacilly
- 🇩🇪Germany DiDebru
I made another MR to resolve the conflicts with https://www.drupal.org/i/3115858 → https://git.drupalcode.org/project/commerce_saferpay/-/merge_requests/5
- 🇫🇷France GuillaumePacilly
@DiDebru thanks for your update, worked fine for me! I added your changes to the MR of this issue, and added another hook to alter payment page initialization data as I need it on my projects.
- Status changed to Needs work
2 months ago 1:36pm 6 February 2025 - 🇨🇭Switzerland berdir Switzerland
Thanks for working on this. Such huge MR is quite challenging to review and handle, but given the stability and (non-)maintenance status of this project, I'm willing to accept it, we're now also in a position where we need some of these updates and will test them.
Per MR comment, my main concern is the extremely long sleep on the notify callback, that's really not an option on a hosting platform such has platform.sh where you have very limited php-fpm processes. Race conditions between notify and return are just a never-ending story, it's really frustrating. this was merged from 🐛 Saferpay webhook faster than customer Active ,
The question is whether the onNotify processing is really required for this or we could move that to a separate issue.
- 🇨🇭Switzerland berdir Switzerland
I see, the new API only has a single return URL. I guess relying on separate vs cancel URL was a problem as you could manually alter it. One option would be to just verify that the payment was a success without storing/placing that information and letting the webhook deal with it.
I've added a setting for the wait time and I removed the locking. A lock is exactly what loadForUpdate() does, and that's now committed in commerce, there's no benefit to having two locks.
- 🇫🇷France GuillaumePacilly
Thanks for the update!
I agree with your approach.
But the single return URL also makes it difficult to properly catch failed or cancelled payments.
I would like to add an Handler to at least catch cancellation from the user and redirect him properly.But I would prefer creating a separate issue and MR for this, based on this one.
Do you think this can be merged soon? Or should we work directly on this MR?