- Issue created by @joelpittet
- Merge request !2Use place transition instead of paid for registration complete → (Closed) created by joelpittet
- Status changed to Needs review
over 1 year ago 6:27pm 10 October 2023 - 🇺🇸United States john.oltman
Thanks for the post @joelpittet. This sounds like a bug in Commerce, it should not fire the paid transition for a free canceled order. I'll see if I can reproduce that at some point. In the meantime, I'd rather add a check for non-canceled state to the Commerce Registration paid transition handler instead of completely removing the handler as you are doing in your MR. Removing it will cause regressions for others.
- 🇨🇦Canada joelpittet Vancouver
@john.oltman You might be correct about a regression but I can't think of one off hand...
The current one is wrong and only fires/dispatches on order presave and only if this data flag
paid_event_dispatchedisFALSEpublic/modules/contrib/commerce/modules/order/src/OrderStorage.php:129Which could make for some hard to track down bugs IMO.
The only regression that comes to mind is if someone rewrites the service to fix this.
This would suggest (from my understanding) that checkout complete is on the
commerce_order.place.post_transitionis the right event from D7 Commerce 1.
https://docs.drupalcommerce.org/commerce2/developer-guide/adapting-from-...A bit worried about just checking status on the paid
- 🇺🇸United States john.oltman
Thanks for this additional info. Let me dig into the good points you made a bit more. May need several days to find time but should have thoughts back to you by early next week.
- 🇨🇦Canada joelpittet Vancouver
Actually it does sound like a bug in commerce too, now that I think about it… I went back and forth on this in my head.
Now I think it’s both a bug here and commerce.
Like you said it shouldn’t fire on drafts but since it’s a presave it might be a chicken/egg scenario.
- 🇺🇸United States john.oltman
@joelpittet would you be able to test the Commerce fix posted in the related issue I opened there. Here is the patch from my MR in that issue.
https://git.drupalcode.org/project/commerce/-/merge_requests/184.diff
Apply that to the commerce module, and the issue you reported here should disappear.
Thanks!
- Status changed to Closed: works as designed
about 1 year ago 4:58am 29 November 2023 - 🇺🇸United States john.oltman
The MR posted to Drupal Commerce was merged and will be in the next release of Commerce. Marking this "works as designed" since that fix will solve this issue.