Ok, so here is a bit of a theoretical problem, but with exact steps to reproduce.
First, let's talk about order refresh. Default order refresh behavior is to refresh if we are loading an order, and if the order is in state draft, and the changed time is older than 300s.
Now, let's look at the code that handles the refresh.
/**
* {@inheritdoc}
*/
protected function postLoad(array &$entities) {
if (!$this->skipRefresh) {
/** @var \Drupal\commerce_order\Entity\OrderInterface[] $entities */
foreach ($entities as $entity) {
$explicitly_requested = $entity->getRefreshState() == OrderInterface::REFRESH_ON_LOAD;
if ($explicitly_requested || $this->orderRefresh->shouldRefresh($entity)) {
// Reuse the doPostLoad logic.
$entity->setRefreshState(OrderInterface::REFRESH_ON_SAVE);
$entity->save();
}
}
}
return parent::postLoad($entities);
}
Ok, so if the order needs refresh, it will set a new state, and save the order.
Now, let's imagine this scenario:
A drush job is running on a server, doing some checks on orders. To do these checks, it has to load them, thus of course triggering postLoad. postLoad will be triggered even if the order was fetched from the cache.
Now let's assume a customer completes their order, triggering an entity save just after the drush job has loaded the draft order from the cache. And lets assume order refresh just hits. Then the drush job will trigger the entity save with its own cached (outdated) entity, setting the order back to draft even though the user has just saved the order as the state "fulfillment".
The problem being we should not rely on something that can be outdated for saving.
There are also other ways to trigger it, for example if you trigger somehow an ajax request loading an order in one tab, while completing the order in the other tab.
Ok, so to reproduce in an easy way, one can do this:
- Add something to the cart.
- Proceed to checkout.
- Mark the order id. For example 42.
- Proceed to off-site payment (not a requirement, just makes the timing a bit easier)
- Set a breakpoint in orderStorage before the shouldRefresh logic.
- Make sure the logic for shouldRefresh triggers (for example by setting it to 5 seconds).
- Start a drush script (triggering the breakpoint) that does this:
// Order number should be the one from step 3.
$order = \Drupal\commerce_order\Entity\Order::load(42);
- Finish the payment.
- Let the breakpoint continue.
The order is now back at draft, and it has no order number or placed time any more.
Suggested solution: Clear the entity cache and re-load the entity before we attempt to refresh the order?
Active
2.0
Checkout
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.
So going back to this, my recommendation to skip the refresh on load... If there is no intent to save the order would be to call the loadUnchanged() method.
Also, Now that
🐛
The changes made to the order on the onNotify method are not applied on the onReturn method
Fixed
is in, we now have a loadForUpdate() method that you can be called if needed. So I wonder if we should close this?