| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | system.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (duplicate) |
Issue Summary
I have a trigger for "After saving a new post" and "After saving an updated post". The action is to send mail to my address. The mail subject begins with: "User: %username".
When a user (UserB) modifies a book page created by UserA, I receive a mail titled: "User: UserA".
The result of %username seems confusing now. What it actually stands for now is %authorname.
In my opinion, the expected behavior would be to receive a title "User: UserB" indicating it was UserB who triggered the event. I am not interested in the original author's name, only in who triggered this event.
If the placeholder %username is not available for indicating the currently active user triggering the event, could we have a different variable for this? ("%activeuser"). Anyway, there should be a clear separation between the two.
If nothing is done (code-wise) to this issue, at least the %username placeholder could be documented better so the admin knows not to expect the name of the triggering user.
Comments
#1
Relevant code is below. I agree that this should be changed, although it is up to Gabor whether it can still get into D6.
<?php
// system.module
$recipient = $context['recipient'];
if (isset($node)) {
if (!isset($account)) {
$account = user_load(array('uid' => $node->uid));
}
if ($recipient == '%author') {
$recipient = $account->mail;
}
}
if (!isset($account)) {
$account = $user;
}
$language = user_preferred_language($account);
$params = array('account' => $account, 'object' => $object, 'context' => $context);
if (isset($node)) {
$params['node'] = $node;
}
if (drupal_mail('system', 'action_send_email', $recipient, $language, $params)) {
watchdog('action', 'Sent email to %recipient', array('%recipient' => $recipient));
}
else {
watchdog('error', 'Unable to send email to %recipient', array('%recipient' => $recipient));
}
}
/**
* Implementation of hook_mail().
*/
function system_mail($key, &$message, $params) {
$account = $params['account'];
$context = $params['context'];
$variables = array(
'%site_name' => variable_get('site_name', 'Drupal'),
'%username' => $account->name,
);
?>
I propose to change this to: (no patch unless this goes through)
<?php
// system.module
$recipient = $context['recipient'];
if (isset($node)) {
$author = user_load(array('uid' => $node->uid));
if ($recipient == '%author') {
$recipient = $author->mail;
}
}
if (!isset($account)) {
$account = $user;
}
$language = user_preferred_language($account);
$params = array('account' => $account, 'object' => $object, 'context' => $context);
if (isset($node)) {
$params['node'] = $node;
}
if (drupal_mail('system', 'action_send_email', $recipient, $language, $params)) {
watchdog('action', 'Sent email to %recipient', array('%recipient' => $recipient));
}
else {
watchdog('error', 'Unable to send email to %recipient', array('%recipient' => $recipient));
}
}
/**
* Implementation of hook_mail().
*/
function system_mail($key, &$message, $params) {
$account = $params['account'];
$context = $params['context'];
$variables = array(
'%site_name' => variable_get('site_name', 'Drupal'),
'%username' => $account->name,
);
?>
system_mail does not use $account anywhere else, so changing $account to be the user rather than the author does not make any other difference. I propose that in D7, both $author and $user variables become available; in DRUPAL-6 this would require string changes.
#2
Enclosed is an untested patch with the changes proposed by Arancaytar.
#3
Moving this to the D7 queue.
#4
This patch still applies, but as I stated earlier, this should be improved since in D7 we can still change strings.
#5
Subscribing.
An option would also be to keep a token for the node author, in case someone wants to use that string. Something like this in
system_mail:<?php$variables = array(
'%site_name' => variable_get('site_name', 'Drupal'),
'%authorname' => $author->name,
'%username' => $user->name,
);
?>
#6
Seems relevant still
#7
This has been fixed in #113614: Add centralized token/placeholder substitution to core: Add centralized token/placeholder substitution to core.
Marking this as duplicate.