- Issue created by @andyg5000
- Status changed to Needs review
about 2 years ago 7:10pm 13 March 2023 - Status changed to Needs work
about 2 years ago 10:06am 30 March 2023 - 🇮🇱Israel jsacksick
I don't think this change is acceptable as is... As this would basically break orders refreshed manually... Or from outside of the checkout context basically which is why the tests are failing.
What is the usecase exactly? Because in theory, any change made on a draft order can potentially be blown away by the refresh (this applies to prices, promotions, taxes etc).
- 🇺🇸United States lisastreeter
This issue caught my attention because we allow admins to change order emails and we continue to perform order refresh after an order is placed. So we essentially leave Drupal Commerce alone to do its (standard) thing for customers shopping on the website but for admin users, we have them "place" a new order right away. Then for them, the experience of editing an order a customer placed vs an order they placed is identical. We control order refresh triggers through custom code. But we're B2B and changes to already-placed orders are not uncommon. All that being said, this is how we modify
orderRefresh
to keep the email from getting reset by refresh after a change by an admin:Replace:
if ($customer->isAuthenticated())
With:if ($customer->isAuthenticated() && $order->getState()->getId() == 'draft')
We've had our custom patch in production since last summer without any problems/conflicts with updates. Perhaps as @jsacksick suggests, merging this patch would just break too many things; it may be better as a custom patch within your project.
- 🇮🇱Israel jsacksick
@lisastreeter: The logic is in the order refresh service, which only acts on draft orders already?
- 🇺🇸United States lisastreeter
@jsacksick -- yes, my patch wouldn't affect anything at all for a standard Commerce install. (Which is why I didn't bother creating an issue or suggesting the patch as an update.) But in my custom code, I've got this line in several different places, affecting non-draft orders:
$order->setRefreshState(OrderInterface::REFRESH_ON_SAVE);
I've found that Order Refresh works perfectly fine for non-draft orders. The only thing that was getting messed up for placed orders was the contact email address. That little one-line change fixed it without affecting draft orders at all.
Admin users are allowed to change prices/quantities/shipping methods/items ordered and override taxes for already-placed orders. When they do, I need to run order refresh manually.
My personal opinion for this issue is that we should probably leave Order Refresh unaltered--people with complex sites may already be doing custom things related to order refresh.