The feature request is still relevant in 4.0.x.
Re-rolled patch #14 against Drupal 10.2.3
@jsacksick Good point !
I rebased the branch on the latest 8.x-1.x branch, updated the code with the destination service and fixed the functional test (which seemed unrelated).
Thank you for the updated code @viren18febS .
Unfortunately, it doesn't seem to fix my issue. The langcode is correctly set on the address but it is still the current langcode that it is used by Mailer::doSend
. In the context of a legacy email, the __disable_customize__
parameter is set and the langcode processing in the Mailer::doSend
method is ended early intentionally, no address is parsed. The address langcode is correct but not the email langcode.
We could prevent the setting of the __disable_customize__
parameter in LegacyEmailBuilder::createParams
, but then the account will also be updated/processed in Mailer::doSend
which may have unwanted side effects.
We could otherwise split the disabling parameter into two: __disable_customize_langcode__
and __disable_customize_account__
. This would offer a fine-grained way of handling overrides but will require additional conditions in the process loop.
Or we could consider that __disable_customize__
only excludes account customization and still gather the langcode from the to addresses.
Here is a patch proposition.
Aerzas β created an issue.
The proposition fixed (wrong condition).
The proposition as a patch.
Aerzas β created an issue.
Adding re-rolled patches against the new 3.x and 4.x versions
Thank you for your contribution @heshamkh
I am not sure this is the right way to do it, to propose another field widget to solve this requirement. They will basically do the same thing in a slightly different way, why not merge them together?
Could you please also propose an interdiff file when you suggest changes so that we can clearly see what are your additions please?
I rerolled my initial patch against 8.x-2.37.
Aerzas β created an issue.
Here is the update as a patch
Aerzas β created an issue.
Update patch against r4032login
2.2.1
The patch representing the pull request.
Aerzas β created an issue.
Aerzas β made their first commit to this issueβs fork.
@jsacksick That sounds like a good solution indeed, thanks for the tip.
Let's close this issue because the solution should be developed in our custom code base.
@jsacksick I get your point but it seems realistic to programmatically update an order with further information once a transition has passed.
- Using the preTransition to avoid saving the entity twice could be too optimistic as the transition could still fail and some processing like sending emails cannot be reverted.
- We could listen to the entity update hook or event (like OrderEvents::ORDER_UPDATE
), as the entity update is dispatched after the postTransition is resolved, and compare the original entity state, this would produce the same result but it seems less optimized to listen to all update events to catch a single transition. We would still save the entity twice though.
- We could save some data in a separated storage (Drupal state, dedicated table, ...), we could still get it from the entity via computed field if needed. This is probably the intended way but it may feel overkill for very simple updates like storing an email timestamp or third-party service reference.
We prepared a patch to prevent the same transition to be applied twice (provided as a reference) but I guess we'll have to investigate the third option.
Although the tests have passed, this patch would prevent the post transition subscribers to known the initial state of the transition which could break some existing listeners like OrderEventSubscriber::onOrderPostTransition
in the commerce_order
module.
Here is the associated patch.
Aerzas β created an issue.