Problem/Motivation
"Commerce MultiSafepay payments" in API calls to MSP gateway uses as order id $order->getOrderNumber() if is set, $order->id() otherwise
for example this line from OrderHelper::logMSP() and from OrderHelper::createOrderData()
$orderId = $orderNumber = $order->getOrderNumber() ?: $order->id();
But the OrderNumber if assigned via EventSubscriber in modules\contrib\commerce\modules\order\src\EventSubscriber\OrderNumberSubscriber.php setOrderNumber()
and it is triggered via commerce_order.place.pre_transition event
which is invoked either :1) order reached 'complete' step of checkout flow (commece_checkout module) 2) either if order way 100% payed (commece_payment module, invoked in result of incoming request from MSP to GatewayStandardMethodsHelper::onNotify())
so on practice (depending on MSP gateway response speed / Drupal slowness) I am getting next situation:
- Checkout step payment redirects to MSP offsite form where user inputs data and hits pay button. StandardPaymentForm::buildConfigurationForm() here $this->mspOrderHelper->createOrderData($form, $payment); is sending data with $order->id(), because order is not completed and getOrderNumber() is empty
- commece_checkout module changes order state to completed, because it passed reached 'complete' step/pane, and $order->getOrderNumber() is assigned
- MSP gateway invokes notification_url in Drupal and GatewayStandardMethodsHelper::onNotify() which calls OrderHelper::logMSP() which uses getOrderNumber() but order was registered with $order->id() on MSP on step 1
and story ends up with PaymentGatewayException: 1006 : Invalid transaction ID , the payment status is not updated and order is not payed in Drupal but is payed on fact
If Drupal is slow enough not reach checkout step 'complete' or this step is not next after payment - then all works fine because $order->id() temporary is still used.
Steps to reproduce
I think is clear from text above.
Proposed resolution
Use only $order->id() everywhere. Also getOrderNumber() may overlap between different bundles with time, for example default orders and recurring orders.
Will publish patch ASAP.
In long perspective is even better to use UUID if enabled. But this is not a subject at the moment.
Remaining tasks
Consider there are few sites report using v3.0 module. Seems 6 do, 3 of which are probably me/we/my_company.
Rest 3 sites seems will not feel any changes, as 99.9% their payed orders were registered with $order->id() on MSP and we gonna use this field as the only one after patch.
User interface changes
none
API changes
none
Data model changes
none
