Allow to customize Sender

From https://www.drupal.org/project/symfony_mailer/issues/3388098: →

AdamPS: However I have my doubts as

1) The description for the site mail field says this:

The From address in automated emails sent during registration and new password requests, and other notifications. (Use an address ending in your site's domain to help prevent this email being flagged as spam.)

2) AFAIK it isn't possible to configure sender in the GUI in other similar modules

While item 1 has its merits, I don't think item 2 would be of any importance. :-)

Actually, the code allows it, so I can't really see any reason not to allow from the UI as well.

One use case is to use the translated site_name configuration. The code https://git.drupalcode.org/project/symfony_mailer/-/blob/1.x/src/Address... hardcodes the Sender header as default language Site name.

The Sender should come from site configuration. If you want to change it, set "site mail" in the Basic site settings - that's exactly what that setting is for, clearly documented in the description. So why is another setting needed?

#4 I agree is a bug. This issue GUI setting could be a workaround. The correct fix is a code change to use the correct translation (fix the code) - please can you raise a separate issue?

Created an issue for #4: 🐛 Address::create should take langcode into account Active

@AdamPS

The Sender should come from site configuration. If you want to change it, set "site mail" in the Basic site settings - that's exactly what that setting is for, clearly documented in the description. So why is another setting needed?

There are use cases outside of this, e.g. you can have more than one commerce store with different email addresses. This is a core commerce feature, see commerce_store_mail_alter

function commerce_store_mail_alter(&$message) {
  if (substr($message['id'], 0, 9) == 'commerce_' && empty($message['params']['from'])) {
    /** @var \Drupal\commerce_store\CurrentStoreInterface $current_store */
    $current_store = \Drupal::service('commerce_store.current_store');
    $current_store = $current_store->getStore();
    if ($current_store) {
      $message['from'] = $current_store->getEmailFromHeader();
    }
  }
}

I have a requirement to set a different sender for certain emails, so I needed to be able to change this via policy. I added a simple plugin to custom code to do so, I've ported it here as part of Symfony Mailer itself if this is useful.

I created a similar issue in 📌 Webform from address not set as sender address when using symfony_mailer Needs review .
I also add a MailAdjuster, which automatically takes the primary from address and sets it as a sender.

I would propose to combine the plugins to provide more configuration:
- Take the primary from address or
- Manually set a sender email.

Or should we add separate adjusters?

I suppose a third option is to add a checkbox to FromEmailAdjuster which also sets the sender?

For me, this solution is a clearer UX than 📌 Webform from address not set as sender address when using symfony_mailer Needs review .

1) If do this we need to provide a clear warning in the UI, for example in a description text. Firstly normally don't use this, set the site address instead. Secondly make sure to use a valid domain so as to avoid getting classed as spam - we could copy the wording from core.

2) Message.php will anyway add a sender based on From. So all we need to do is set the sender to blank. This ensures that it copies the final from - rather than the current value which might later be altered by a hook.

3) For a new feature, please add a test.

I adjusted the code for 2.x. Added the description text copied from Drupal core and tests.

I don't understand point 2 from comment #14. Can you explain more what is needed?

Thanks.

1) #14.1 has two parts and you did the second part. Please also add something like this: 'The sender defaults to the site email address configured in "Basic site settings". You should only use this adjuster if you need a different value.' It's becoming quite long, so I suggest a separate form field, something like $form['warning'] = XXX (this also saves duplicating the original description).

I don't understand point 2 from comment #14

I also find it confusing😃. I believe I had intended #14 be a reply to #13: add an checkbox to FromEmailAdjuster to also set the sender. I was concerned about the case that both "from" and "sender" are present, and someone changes one but forgets the other. My point was that if this option is set, then we could delete the default sender and the value would be copied from 'from'.

Now that you have written SenderEmailAdjuster I'm willing to stick with it. However build() is wrong - it misses 2 of the 3 cases in the parent class. Potentially a neat solution is to enhance BaseEmail::setAddress() also to accept sender. Then we don't have to override the AddressAdjusterBase::build() method at all, and it allows removing various bits of special case code in BaseEmail (and assertSender() can call assertAddress()). It requires a small adjustment to Email::getSymfonyEmail() - inside the loop we check for sender, and do something like $this->inner->sender($value[0]). If you make a start then I can add suggestions as needed.

OR if you are enthusiastic to switch to the alternative solution, add a checkbox to FromEmailAdjuster then I can instead comment about that.

I've thought some more and I like SenderEmailAdjuster. It's consistent with the others (else who knows to look for sender inside from), and it means that if a site actually needs from and sender different for some reason then they can.

Expanding on the previous comment, I believe the key to making an adjuster for Sender is to make it work like the other addresses.

1. Add sender to BaseEmail::$addresses initialiser and remove separate $sender variable
2. Rewrite BaseEmail::setSender/getSender to use setAddress/getAddress. Add a check in setAddress() to assert/throw if $name == sender && count($addresses) > 1.
3. In Email::getSymfonyEmail() remove the call to $this->inner->sender() that is outside the loop and instead add it back inside the loop with if ($name == sender) {...} else {}.
4. MailerTestTrait::assertSender() can now be written in terms of assertAddress().

Now senderEmailAdjuster doesn't need to override build().

Thanks again

Much clear now

I rebased the branch and committed the refactoring following your guidance. Thanks!

rodrigoaguilera → changed the visibility of the branch 3388651-allow-customize-sender to hidden.

rodrigoaguilera → changed the visibility of the branch 3388651-allow-customize-sender-2.x to hidden.

rodrigoaguilera → changed the visibility of the branch 3388651-allow-customize-sender-2.x to active.

rodrigoaguilera → changed the visibility of the branch symfony_mailer-3388651-3388651-allow-customize-sender-2.x to hidden.

Great thanks