- 🇮🇱Israel jsacksick
Reviving this, let's create an MR, we also need tests.
Let's unset the data flag to make it more like a per request flag instead of a global flag that wouldn't allow resending the order receipt manually.
We can unset the flag from the order receipt subscriber.
I'm also wondering if we should update the order type method to check for the order flag as well, but the method doesn't accept an order, so that would be a breaking change. - 🇳🇴Norway zaporylie
One more note here because something similar came up in one of the projects I've been working on recently. In their case, some orders would be created programmatically, and in such a case, all following messaging is supposed to be suppressed. So in that case, the flag would have to be permanent.
So my proposal here is to change from using Boolean values to have a helpful CONST defined on the MailerHelper and have the possibility to set skip_order_receipt to PERMANENT, and in such a case, do not reset the value. We could also use that value to hide the action link. Furthermore commerce_shipping could take advantage and suppress shipment confirmation if needed (TBD).
- 🇮🇱Israel jsacksick
I'm not particularly in favor of making the flag permanent, as you'd never be able to send the email from the UI. Plus you might need to use the order receipt resend form to send the order to another email (once that feature lands, See ✨ Allow specifying an email to send the order receipt to Active ).
If you're explicitly opening the form to resend a receipt, we shouldn't prevent the user from doing that. By MailerHelper are you referring to the OrderReceiptMail service?
Also if we make the flag a non boolean and it can support more than 2 states, we should probably make it an enum.
But maybe we'd need a more global flag that'd be respected by commerce_shipping too for the shipment confirmation (instead of having a separate flag for this).
So in this case, we'd be more looking at a flag that can be set for programmatically placed orders I suppose, rather than what we're trying to solve here (i.e. the place transition applied from the UI);