Dispatch the ORDER_PAID event for free orders when OrderStorage is destroyed

Open on Drupal.org →

Created on 14 November 2023, 8 months ago

Updated 22 February 2024, 4 months ago

Commerce Core is inconsistent in when it dispatches the commerce_order.order.paid event relative to the commerce_order.place.[pre|post]_transition events for free vs. paid orders.

This is because as a free order progresses through checkout, OrderStorage::doOrderPreSave() will dispatch the order paid event synchronously for free orders once they leave the draft state (i.e., when the order redirects to the complete step and the place transition is applied), after which the order placement events will be dispatched.

A paid order, on the other hand, will still appear to be unpaid at this point, because the order's total_paid field value isn't updated until the PaymentOrderUpdater class is actually destroyed. This means the order will be placed first, then as the page request is ending, it will be have its total paid updated, the order will be saved, and then the order paid event will be dispatched.

In other words, the order of these events is:

Free order: order paid -> order placed
Paid order: order placed -> order paid

This isn't a huge problem in and of itself, but it does create inconsistencies in what subscribers of the order paid event can expect. Certain logic is tied to order placement by default, like order number generation, so free orders may be triggering an order paid event with incomplete data relative to paid orders. Given these are likely the edge cases for sites that have a mixture of free vs. paid orders, this issue can be tricky to track down or work around.

My proposal to resolve this is to add a similar $updateList of order IDs to OrderStorage and use a destructor to dispatch the paid event instead of doing it synchronously. This will synchronize the execution of events with paid orders. My only concern is the potential drawbacks ... can we safely make this change in a minor release?

📌 Task

Status

Active

Version

2.0

Component

Developer experience

Created by

🇺🇸United States rszrama

Live updates comments and jobs are added and updated live.

Sign in to follow issues

Comments & Activities

Issue created by @rszrama
Comment 4 months ago →
🇺🇦Ukraine khiminrm
I’ve spent some time to try to implement the suggested fixes.

But with the __destruct() method it doesn’t work in OrderStorage.

I’ve got the error on line if ($this->lockBackend->acquire($lock_id)) { in loadForUpdate() $order_id)Drupal\Core\Database\StatementWrapperIterator::__construct(): Argument #2 ($clientConnection) must be of type object, null given, called in /var/www/html/web/core/lib/Drupal/Core/Database/Connection.php on line 557

Maybe related to https://www.drupal.org/project/drupal/issues/1924166 →

Also I tried to add custom service with needs_destruction tag and with destruct() method and use it inside the OrderStorage class. But it didn’t work also but with another reason - in the desturct() method the variable with the list of the orders was empty, but I saw in debug that the order ID value was added to the array.

Can’t find properly worked solution.

Looks like we need to resave free order so place transition is applied first and only then dispatch the paid order event on second save. But if free order is created programmatically with other than ‘draft' or ‘canceled’ state then resaving the order doesn’t make sense as ‘placed' transition event shouldn’t be dispatched.

contrib.social Blog FAQ Discussions

Production build 0.69.0 2024