Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.### Problem/Motivation
http://api.drupal.org/api/drupal/includes--mail.inc/function/drupal_mail/8
$message = array(
'id' => $module . '_' . $key,
should be
$message = array(
'id' => $module . '__' . $key,
That would make it easier to identify and alter emails, and could also avoid possible id collisions.
### Proposed resolution
### Remaining tasks
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | 1237720-nr-bot.txt | 149 bytes | needs-review-queue-bot |
Issue fork drupal-1237720
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:
changes
Comments
Comment #1
oadaeh CreditAttribution: oadaeh commentedI'm trying to understand how creating every mail id with two underscores will make it easier to identify e-mails, alter e-mails, and avoid id collisions more than creating every mail id with with one underscore.
Would you please elaborate?
Comment #2
Pasquallefor example
in hook_mail_alter you can alter mails by id.
If you try to alter all mails created by your module you may use
but what if there is another module called "mymodule_other", then you will also alter mails of that module, which is not desired..
Comment #3
oadaeh CreditAttribution: oadaeh commentedSomething in my gut says you're doing it wrong, and I don't think you're going to get much support, but I could be wrong, so I'm going to leave the issue as is and let a core developer make the determining decision.
Comment #12
jungle+1 to this.
Comment #14
PasqualleIn D9 MailManager::doMail() has separate parameters for module and mail key, so it is possible to get those values in mail alter.
But I still think, the id should be with double underscores to avoid possible id collisions.
mymodule_other__key vs mymodule__other_key
Comment #16
PasqualleThis should probably go to Drupal 10 as I would consider it as a breaking change.
Comment #17
PasqualleComment #18
Kristen PolI'm not clear why this was changed to a bug. This seems like a feature request IMO. @jungle?
Comment #19
Kristen PolIt's not clear above but tests did pass even though #15 shows a failure. I assume @Pasqualle or someone reran the tests. You can see the success with a later time than the failure at the bottom of https://www.drupal.org/pift-ci-job/2006986.
But, this was tested on 9.2... do we need to test on 10?
Comment #20
longwaveI don't see how we can make this change in a backward-compatible way, or nor what bug it fixes - this just seems to be more disruptive than helpful. The IS mentions "possible ID collisions" but is this actually a real issue?
Comment #21
jungleAfter checking into details of the related code, for the use case mentioned in #2, the
modulekey of$messagecould be relied on, instead of depending on theidkey as #14 pointed.I'd say this is a won't fix, neither a bug nor a feature request.
Thanks!
Comment #22
Kristen PolThanks for your input and workaround, @jungle. Afaik, we normally keep the Bug Smash Initiative tag even when switching to a support request so we know the bugsmash team worked on it.
Comment #23
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #24
cilefen CreditAttribution: cilefen as a volunteer commentedI am closing this based on #21.