Problem

It is possible for an order to be placed more than once. In particular, this can happen inadvertently if you save an order within an place pre-transition event subscriber, as this triggers the pretransition event to be dispatched again.

However, according to @Bojanz

Everything in Commerce assumes that an order can only be placed once.
Expect critical issues all over the ecosystem if you place an order twice.

Todo

Figure out how to catch this and crash in an informative way.

Original report

It is possible for an initial order's place transition to be triggered multiple times. One obvious way is if you have a workflow that allows placed orders to go back from completed to draft, or where the place transition can also go from canceled to complete as well as draft to complete. But even without such possibly illegitimate workflows it can happen. In my case I created an event subscriber for the place pre-transition event, with a higher priority than the commerce subscriber to the same event, and assigned an order to a user in it.

The result was multiple subscriptions for the initial order, probably because the order assigning triggered an order save and fired the place event again. Possibly this is a bug in state_machine, but maybe recurring should be wise to the possibility too. One solution would be for SubscriptionStorage::createFromOrderItem() to refuse to create a duplicate subscription for the same order item.

Comments

jonathanshaw created an issue. See original summary.

bojanz’s picture

bojanz commented

Everything in Commerce assumes that an order can only be placed once.
Expect critical issues all over the ecosystem if you place an order twice.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

If that's the case, is there a way we could use state machine guards to prevent recursive triggering of the place transition?

All I did was assign an order to a user in an event subscriber reacting to that same orders place event.

Recursiveness is a tricky area, I've been bitten by this with entity pre/post save hooks.

bojanz’s picture

bojanz commented

Saving an order within the presave event is supposed to crash the system, curious that you didn't run into #2961440: Error _editor_get_file_uuids_by_field() .

I am not yet sure what the best way is to catch this, and crash in a more informative way.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Project: Commerce Recurring Framework » Commerce Core
Version: 8.x-1.x-dev » 8.x-2.x-dev
Component: Code » Order
Issue summary: View changes

So maybe a more general Commerce issue is appropriate here.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Related issues: +#2961440: Error _editor_get_file_uuids_by_field()
bojanz’s picture

bojanz commented
Title: Duplicate subscriptions created if place transition is triggered multiple times » Throw a nice exception when saving an order inside a pre_save/post_save event subscriber

Better title.

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
commented

So what is the proper way to change an order's state after the order has been submitted?

I have the following situation:

  1. 1A user places an order on the site - its state is "validation".
  2. I have an export process that needs to take all new orders and immediately send them to an external API for fulfillment. If that process is successful I want to then transition the order to "complete".
  3. I am currently triggering my custom code to the "commerce_order.place.post_transition" event, but my code involves a $order->save(), so...
  4. I have just gotten bit by #2961440: Error _editor_get_file_uuids_by_field() . What the proper solution is in my situation?

thanks,
-mike

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

I resolve my need to save the order by calling the code that does this from the onStepChange() method of my CheckoutFlow plugins.

It's annoying but it works.

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
commented

With some help from @bojanz and @mglaman in #commerce, I found a solution I'm very happy with using [DestructableInterface](https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Destructa...) to call my custom code (which includes $order->save()) when my event subscriber object is destroyed.

Here's [a gist](https://gist.github.com/mglaman/2925bf398a35cc9c7fc1008ee65a7280) that @mglman provided me that unlocked it for me.

-mike

dunebl’s picture

dunebl
Primary language French
commented

@ultimike thank you for sharing this code (and also @bojanz and @mglaman).
Unfortunatly it doesn't work for my use case.
I am not asking here a support, (I could solve by another mean) but I am posting to share my finding.
Here is my use case: applying another transition inside a state_machine post_transition event subscriber
1-[Button click] $stateItem->applyTransitionById('id1')
2-[Inside state_machine post_transition event subscriber] if ($something){$stateItem->applyTransitionById('id2');$entity->save();}

The destruct() function is well called, but at the end, the only applied transition is 'id1'