Problem/Motivation

When using LegacyEmailBuilder, the email langcode is always set to the current langcode. Since a langcode is provided to the mail method on the MailManager, we should use that langcode.

Proposed resolution

Use the legacy message langcode during the processing in Mailer.

CommentFileSizeAuthor
#2 symfony_mailer-set_langcode_with_legacyemailbuilder-3413081-2.patch604 bytesaerzas

Issue fork symfony_mailer-3413081

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Aerzas created an issue. See original summary.

aerzas’s picture

aerzas commented
Status: Active » Needs review
StatusFileSize
new symfony_mailer-set_langcode_with_legacyemailbuilder-3413081-2.patch604 bytes

Here is a patch proposition.

adamps’s picture

adamps commented
Status: Needs review » Needs work

Thanks for the issue and patch. Strange, I thought I had written code for this already😃.

The issue is only about LegacyEmailBuilder so the fix should also be in the legacy code. I believe we can fix it in LegacyMailerHelper::emailFromArray(). On line 141 there is a call to $email->setAddress(). We can pass a second parameter of $message['langcode']. This should be only for the to address.

This project now uses merge requests not patches please.

viren18febS made their first commit to this issue’s fork.

viren18febS opened merge request !83

viren18febs’s picture

viren18febs commented
Status: Needs work » Needs review

This project now uses merge requests not patches please.

Please review.

adamps’s picture

adamps commented
Status: Needs review » Needs work

Thanks, I mean pass langcode as second parameter to parseAddress()

viren18febs’s picture

viren18febs commented
Status: Needs work » Needs review

I have updated langcode as second parameter to parseAddress(), please review.

adamps’s picture

adamps commented

That looks good to me. @Aerzas please can you test?

aerzas’s picture

aerzas commented
Status: Needs review » Needs work

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.

adamps’s picture

adamps commented

Good point, thanks @Aerzas. How about this?

- LegacyEmailBuilder creates the To address with the current langcode and also current account. Add an extra param to parseAddress.
- Remove all code for __disable_customize__

yorder.be’s picture

yorder.be commented

Has there been any progress on this issue?
Seems quite the priority since this affects all Core user mails.
Or is there another workaround to be able to send the mails in the correct language?

aerzas’s picture

aerzas commented

@yorder.be The patch I proposed fixes the issue in the meanwhile.

aerzas’s picture

aerzas commented

@AdamPS Thank you for your feedback, I'll work on a new MR as soon as I can.

adamps’s picture

adamps commented
Version: 1.4.0 » 2.x-dev
Issue tags: +Novice

__disable_customize__ has now been removed from 2.x. It should now be a 1 line fix to LegacyMailer::send() to pass $message['langcode'] as the 2nd parameter.

koustav_mondal’s picture

koustav_mondal commented
Assigned: Unassigned » koustav_mondal

koustav_mondal opened merge request !139

koustav_mondal’s picture

koustav_mondal commented
Assigned: koustav_mondal » Unassigned
Status: Needs work » Needs review

@adamps Made the changes. Previous MR was forked form 1.4.0 and LegacyMailer class was not present in that specific version. So, I pulled the latest 2.x-dev and raised new MR-139 and made the changes in that MR. Kindly review it.

adamps’s picture

adamps commented
Status: Needs review » Needs work

Thank please see the comment in the MR

koustav_mondal’s picture

koustav_mondal commented
Status: Needs work » Needs review

Added 2nd parameter to parseAddress().

adamps’s picture

adamps commented

Great, now it needs someone to test. A manual test would be OK. Set up a multi-language site, configure a user with a non-default language, send an email with template text that depends on language. Test without the fix the language is wrong, with the fix it is right.

debrup’s picture

debrup commented

I was testing this issue as mentioned by @adamps.

  • Set up a multilingual site with a user configured with a non-default language.
  • Used hook_mail() and mail() method to send mail during user creation (for using LegacyEmailBuilder).
  • However the correct language template is being sent to the receiver even without the fix based on the user's langcode.

Am I doing it correctly? If not, then mention where and how the error occurs and how to reproduce it. Any help is appreciated. Thanks in advance :)

adamps’s picture

adamps commented

Thanks @debrup

I have one idea - make sure that receiver langcode is not the default.

debrup’s picture

debrup commented

@adamps I have created a new user set to a non-default language. When their account gets created the mail is sent with the corresponding template in the langcode set for that user.

adamps changed the visibility of the branch 3413081-set-langcode-with to hidden.

adamps’s picture

adamps commented
Issue tags: -Novice

Also please make sure that the current account that you are logged in with has a different langcode from the one on the new user.

adamps’s picture

adamps commented
Status: Needs review » Needs work

Sorry I realise anyway that the fix is incomplete, my mistake. We need to

  1. Remove reference to __disable_customize__ in Mailer::doSend() and LegacyMailer::send()
  2. Add a 3rd parameter to ImportHelper::parseAddress() which is ?AccountInterface $account = NULL and pass the value to new Address() in 2 places.
  3. In LegacyMailer, after the code that we already added, pass a 3rd parameter which is the current account. Add an extra constructor parameter protected readonly AccountInterface $account to fetch this.
adamps’s picture

adamps commented
Assigned: Unassigned » adamps

adamps changed the visibility of the branch 1.x to hidden.

adamps’s picture

adamps commented

This issue is missing "steps to reproduce" and symptoms. I agree with @debrup that it is hard to reproduce. I can confirm that the langcode is wrong in Mailer.php (by adding a debug print), which affects the current langcode during the call to hook_mail. However both user_mail and contact_mail ignore the current langcode, and use message['langcode'] so the emails produced are all correct.

adamps’s picture

adamps commented
Status: Needs work » Needs review

I have updated the MR so it works in my testing, confirmed by debug print. Please review @aerzas.

adamps’s picture

adamps commented
Assigned: adamps » Unassigned

adamps’s picture

adamps commented
Status: Needs review » Fixed

I've now added a test. I'll commit as I'm getting too many issues in progress. Anyone else please do test to confirm.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.