Skip to content

Commit

Permalink
MDL-64148 core: Make email greetings consistent
Browse files Browse the repository at this point in the history
Some email body strings use first names as greetings,
some use full names, and some do not.

Using the first name for greeting makes it consistent and
a bit more "personal".
  • Loading branch information
meirzamoodle committed Jul 30, 2024
1 parent b4f1704 commit 699daaf
Show file tree
Hide file tree
Showing 16 changed files with 200 additions and 20 deletions.
6 changes: 6 additions & 0 deletions .upgradenotes/MDL-64148-2024071508585319.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
issueNumber: MDL-64148
notes:
core:
- message: >-
Add \core_user::get_name_placeholders() to return an array of user name fields.
type: improved
6 changes: 3 additions & 3 deletions enrol/manual/tests/behat/welcomemessage.feature
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Feature: A course welcome message will be sent to the user when they are enrolle
Then I should see "Send course welcome message"
And the field "Send course welcome message" matches value "From the course contact"
And I should see "Custom welcome message"
And the field "Custom welcome message" matches value "Dear {$a->fullname}, you have successfully been enrolled to course {$a->coursename}"
And the field "Custom welcome message" matches value "Hi {$a->firstname}, you have successfully been enrolled to course {$a->coursename}"
And I should see "Accepted formats: Plain text or Moodle-auto format. HTML tags and multi-lang tags are also accepted, as well as the following placeholders:"
And I set the field "Send course welcome message" to "No"
And I should not see "Custom welcome message"
Expand Down Expand Up @@ -87,14 +87,14 @@ Feature: A course welcome message will be sent to the user when they are enrolle
And I open the notification popover
And I should see "Welcome to Course 1"
And I click on "View full notification" "link" in the ".popover-region-notifications" "css_element"
And I should see "Dear First User, you have successfully been enrolled to course Course 1"
And I should see "Hi First, you have successfully been enrolled to course Course 1"
# Login as second user and check the notification.
And I am on the "C1" "course" page logged in as user2
And I should see "1" in the "#nav-notification-popover-container [data-region='count-container']" "css_element"
And I open the notification popover
And I should see "Welcome to Course 2"
And I click on "View full notification" "link" in the ".popover-region-notifications" "css_element"
And I should see "Dear Second User, you have successfully been enrolled to course Course 2"
And I should see "Hi Second, you have successfully been enrolled to course Course 2"

@javascript
Scenario: Students should receive a welcome message if the setting is enabled - Custom message
Expand Down
2 changes: 1 addition & 1 deletion enrol/manual/tests/lib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ public function test_send_course_welcome_message(): void {
get_string(
'customwelcomemessageplaceholder',
'core_enrol',
['fullname' => fullname($student), 'coursename' => $course->fullname],
['firstname' => $student->firstname, 'coursename' => $course->fullname],
),
$message->fullmessage,
);
Expand Down
6 changes: 3 additions & 3 deletions enrol/self/tests/behat/welcomemessage.feature
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Feature: A course welcome message will be sent to the user when they auto-enrol
Then I should see "Send course welcome message"
And the field "Send course welcome message" matches value "From the course contact"
And I should see "Custom welcome message"
And the field "Custom welcome message" matches value "Dear {$a->fullname}, you have successfully been enrolled to course {$a->coursename}"
And the field "Custom welcome message" matches value "Hi {$a->firstname}, you have successfully been enrolled to course {$a->coursename}"
And I should see "Accepted formats: Plain text or Moodle-auto format. HTML tags and multi-lang tags are also accepted, as well as the following placeholders:"
And I set the field "Send course welcome message" to "No"
And I should not see "Custom welcome message"
Expand Down Expand Up @@ -73,7 +73,7 @@ Feature: A course welcome message will be sent to the user when they auto-enrol
And I open the notification popover
And I should see "Welcome to Course 1"
And I click on "View full notification" "link" in the ".popover-region-notifications" "css_element"
And I should see "Dear First User, you have successfully been enrolled to course Course 1"
And I should see "Hi First, you have successfully been enrolled to course Course 1"
# Login as second user and check the notification.
And I log in as "user2"
And I am on "Course 2" course homepage
Expand All @@ -82,7 +82,7 @@ Feature: A course welcome message will be sent to the user when they auto-enrol
And I open the notification popover
And I should see "Welcome to Course 2"
And I click on "View full notification" "link" in the ".popover-region-notifications" "css_element"
And I should see "Dear Second User, you have successfully been enrolled to course Course 2"
And I should see "Hi Second, you have successfully been enrolled to course Course 2"

@javascript
Scenario: Students should receive a welcome message if the setting is enabled - Custom message
Expand Down
41 changes: 41 additions & 0 deletions enrol/tests/enrollib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -1827,4 +1827,45 @@ public function test_update_enrol_plugin_data(): void {
$modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance);
$this->assertEquals($expectedinstance, $modifiedinstance);
}

/**
* Test case for checking the email greetings in various user notification emails.
*
* @covers \enrol_plugin::send_course_welcome_message_to_user
*/
public function test_email_greetings(): void {
global $DB;
$this->resetAfterTest();

// Create course.
$course = $this->getDataGenerator()->create_course([
'fullname' => 'Course 1',
'shortname' => 'C1',
]);
// Create user.
$student = $this->getDataGenerator()->create_user();
// Get manual plugin.
$manualplugin = enrol_get_plugin('manual');
$maninstance = $DB->get_record(
'enrol',
['courseid' => $course->id, 'enrol' => 'manual'],
'*',
MUST_EXIST,
);

$messagesink = $this->redirectMessages();
$manualplugin->send_course_welcome_message_to_user(
instance: $maninstance,
userid: $student->id,
sendoption: ENROL_SEND_EMAIL_FROM_NOREPLY,
message: '',
);
$messages = $messagesink->get_messages_by_component_and_type(
'moodle',
'enrolcoursewelcomemessage',
);
$this->assertNotEmpty($messages);
$message = reset($messages);
$this->assertStringContainsString('Hi ' . $student->firstname, quoted_printable_decode($message->fullmessage));
}
}
4 changes: 3 additions & 1 deletion lang/en/admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,9 @@
$string['locked'] = 'Locked';
$string['lockoutduration'] = 'Account lockout duration';
$string['lockoutduration_desc'] = 'Locked out account is automatically unlocked after this duration.';
$string['lockoutemailbody'] = 'Your account with username {$a->username} on server \'{$a->sitename}\'
$string['lockoutemailbody'] = 'Hi {$a->firstname},
Your account with username {$a->username} on server \'{$a->sitename}\'
was locked out after multiple invalid login attempts.
To unlock the account immediately go to the following address
Expand Down
2 changes: 1 addition & 1 deletion lang/en/auth.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
$string['emailchangepending'] = 'Change pending. Open the link sent to you at {$a->preference_newemail}.';
$string['emailnowexists'] = 'The email address you tried to assign to your profile has been assigned to someone else since your original request. Your request for change of email address is hereby cancelled, but you may try again with a different address.';
$string['emailupdate'] = 'Email address update';
$string['emailupdatemessage'] = 'Dear {$a->fullname},
$string['emailupdatemessage'] = 'Hi {$a->firstname},
You have requested a change of your email address for your account on {$a->site}. To confirm this change, please go to the following web address:
Expand Down
2 changes: 1 addition & 1 deletion lang/en/enrol.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
<li>User last name {$a->lastname}</li>
<li>User course role {$a->courserole}</li>
</ul>';
$string['customwelcomemessageplaceholder'] = 'Dear {$a->fullname}, you have successfully been enrolled to course {$a->coursename}';
$string['customwelcomemessageplaceholder'] = 'Hi {$a->firstname}, you have successfully been enrolled to course {$a->coursename}';
$string['defaultenrol'] = 'Add instance to new courses';
$string['defaultenrol_desc'] = 'It is possible to add this plugin to all new courses by default.';
$string['deleteinstanceconfirm'] = 'You are about to delete the enrolment method "{$a->name}". All {$a->users} users currently enrolled using this method will be unenrolled and any course-related data such as users\' grades, group membership or forum subscriptions will be deleted.
Expand Down
6 changes: 3 additions & 3 deletions lang/en/moodle.php
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@
$string['emailactive'] = 'Email activated';
$string['emailagain'] = 'Email (again)';
$string['emailconfirm'] = 'Confirm your account';
$string['emailconfirmation'] = 'Hi,
$string['emailconfirmation'] = 'Hi {$a->firstname},
A new account has been requested at \'{$a->sitename}\'
using your email address.
Expand Down Expand Up @@ -2501,8 +2501,6 @@
// Deprecated since Moodle 4.5.
$string['commentscount'] = 'Comments ({$a})';
$string['datechanged'] = 'Date changed';
$string['registrationcontactno'] = 'No, I do not want to be contacted by other people';
$string['registrationcontactyes'] = 'Yes, provide a form for other Moodlers to contact me';
$string['newpasswordtext'] = 'Hi {$a->firstname},
Your account password at \'{$a->sitename}\' has been reset
Expand All @@ -2522,3 +2520,5 @@
Cheers from the \'{$a->sitename}\' administrator,
{$a->signoff}';
$string['registrationcontactno'] = 'No, I do not want to be contacted by other people';
$string['registrationcontactyes'] = 'Yes, provide a form for other Moodlers to contact me';
16 changes: 16 additions & 0 deletions lib/classes/user.php
Original file line number Diff line number Diff line change
Expand Up @@ -1589,4 +1589,20 @@ public static function get_users_search_sql(context $context, string $usersearch
];
}

/**
* Generates an array of name placeholders for a given user.
*
* @param stdClass $user The user object containing name fields.
* @return array An associative array of name placeholders.
*/
public static function get_name_placeholders(stdClass $user): array {
$namefields = [
'fullname' => fullname($user),
'alternativefullname' => fullname($user, true),
];
foreach (fields::get_name_fields() as $namefield) {
$namefields[$namefield] = $user->{$namefield};
}
return $namefields;
}
}
6 changes: 5 additions & 1 deletion lib/enrollib.php
Original file line number Diff line number Diff line change
Expand Up @@ -3670,7 +3670,11 @@ public function send_course_welcome_message_to_user(
'course' => $instance->courseid,
],
))->out();
$a->fullname = fullname($user);

$placeholders = \core_user::get_name_placeholders($user);
foreach ($placeholders as $field => $value) {
$a->{$field} = $value;
}

if ($message && trim($message) !== '') {
$placeholders = [
Expand Down
22 changes: 17 additions & 5 deletions lib/moodlelib.php
Original file line number Diff line number Diff line change
Expand Up @@ -6039,7 +6039,10 @@ function setnew_password_and_mail($user, $fasthash = false) {
update_internal_user_password($user, $newpassword, $fasthash);

$a = new stdClass();
$a->firstname = fullname($user, true);
$placeholders = \core_user::get_name_placeholders($user);
foreach ($placeholders as $field => $value) {
$a->{$field} = $value;
}
$a->sitename = format_string($site->fullname);
$a->username = $user->username;
$a->newpassword = $newpassword;
Expand Down Expand Up @@ -6071,6 +6074,11 @@ function send_confirmation_email($user, $confirmationurl = null) {
$data = new stdClass();
$data->sitename = format_string($site->fullname);
$data->admin = generate_email_signoff();
// Add user name fields to $data based on $user.
$placeholders = \core_user::get_name_placeholders($user);
foreach ($placeholders as $field => $value) {
$data->{$field} = $value;
}

$subject = get_string('emailconfirmationsubject', '', format_string($site->fullname));

Expand Down Expand Up @@ -6116,8 +6124,10 @@ function send_password_change_confirmation_email($user, $resetrecord) {
$pwresetmins = isset($CFG->pwresettime) ? floor($CFG->pwresettime / MINSECS) : 30;

$data = new stdClass();
$data->firstname = $user->firstname;
$data->lastname = $user->lastname;
$placeholders = \core_user::get_name_placeholders($user);
foreach ($placeholders as $field => $value) {
$data->{$field} = $value;
}
$data->username = $user->username;
$data->sitename = format_string($site->fullname);
$data->link = $CFG->wwwroot .'/login/forgot_password.php?token='. $resetrecord->token;
Expand All @@ -6143,8 +6153,10 @@ function send_password_change_info($user) {
$supportuser = core_user::get_support_user();

$data = new stdClass();
$data->firstname = $user->firstname;
$data->lastname = $user->lastname;
$placeholders = \core_user::get_name_placeholders($user);
foreach ($placeholders as $field => $value) {
$data->{$field} = $value;
}
$data->username = $user->username;
$data->sitename = format_string($site->fullname);
$data->admin = generate_email_signoff();
Expand Down
18 changes: 18 additions & 0 deletions lib/tests/authlib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -549,4 +549,22 @@ public function test_get_enabled_auth_plugin_classes(): void {
$this->assertEquals(count($plugins), 3);
}

/**
* Test case for checking the email greetings in account lockout notification emails.
*
* @covers ::login_lock_account()
*/
public function test_email_greetings(): void {
$this->resetAfterTest();

$user = $this->getDataGenerator()->create_user();

$sink = $this->redirectEmails(); // Make sure we are redirecting emails.
login_lock_account($user);
$result = $sink->get_messages();
$sink->close();
// Test greetings.
$this->assertStringContainsString('Hi ' . $user->firstname, quoted_printable_decode($result[0]->body));
}

}
41 changes: 41 additions & 0 deletions lib/tests/moodlelib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -5593,4 +5593,45 @@ public static function moodle_array_keys_filter_provider(): array {
],
];
}

/**
* Test case for checking the email greetings in various user notification emails.
*
* @dataProvider email_greetings_provider
* @param string $funcname The name of the function to call for sending the email.
* @param mixed $extra Any extra parameter required by the function.
* @covers ::send_password_change_info()
* @covers ::send_confirmation_email()
* @covers ::setnew_password_and_mail()
* @covers ::send_password_change_confirmation_email()
*/
public function test_email_greetings($funcname, $extra): void {
$this->resetAfterTest();

$user = $this->getDataGenerator()->create_user();

$sink = $this->redirectEmails(); // Make sure we are redirecting emails.
$funcname($user, $extra);
$result = $sink->get_messages();
$sink->close();
// Test greetings.
$this->assertStringContainsString('Hi ' . $user->firstname, quoted_printable_decode($result[0]->body));
}

/**
* Data provider for test_email_greetings tests.
*
* @return array
*/
public static function email_greetings_provider(): array {
$extrasendpasswordchangeconfirmationemail = new \stdClass();
$extrasendpasswordchangeconfirmationemail->token = '123';

return [
['send_password_change_info', null],
['send_confirmation_email', null],
['setnew_password_and_mail', false],
['send_password_change_confirmation_email', $extrasendpasswordchangeconfirmationemail],
];
}
}
7 changes: 6 additions & 1 deletion user/edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,12 @@
$a = new stdClass();
$a->url = $CFG->wwwroot . '/user/emailupdate.php?key=' . $emailchangedkey . '&id=' . $user->id;
$a->site = format_string($SITE->fullname, true, array('context' => context_course::instance(SITEID)));
$a->fullname = fullname($tempuser, true);

$placeholders = \core_user::get_name_placeholders($user);
foreach ($placeholders as $field => $value) {
$a->{$field} = $value;
}

$a->supportemail = $OUTPUT->supportemail();

$emailupdatemessage = get_string('emailupdatemessage', 'auth', $a);
Expand Down
35 changes: 35 additions & 0 deletions user/tests/userlib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -1048,4 +1048,39 @@ public function test_user_get_user_details_groups(): void {
$this->assertCount(1, $result['groups']);
$this->assertEquals($group2->id, $result['groups'][0]['id']);
}

/**
* Verifies that the get_name_placeholders function correctly generates
* an array of name placeholders for a given user object.
*
* @covers ::get_name_placeholders()
*/
public function test_get_name_placeholders(): void {
$this->resetAfterTest();

// Set format for fullname and alternativefullname.
set_config('fullnamedisplay', 'firstname, lastname');
set_config('alternativefullnameformat', 'firstnamephonetic lastnamephonetic');

// Create the target user.
$user = $this->getDataGenerator()->create_user([
'firstname' => 'FN',
'lastname' => 'LN',
'firstnamephonetic' => 'FNP',
'lastnamephonetic' => 'LNP',
'middlename' => 'MN',
'alternatename' => 'AN',
]);

// Add user name fields to $a as an object based on $user.
$a = new \stdClass();
$placeholders = \core_user::get_name_placeholders($user);
foreach ($placeholders as $field => $value) {
$a->{$field} = $value;
}
$this->assertEquals("Hello $a->firstname", "Hello FN");
$this->assertEquals("Bonjour $a->fullname", "Bonjour FN, LN");
$this->assertEquals("Privyet $a->alternativefullname", "Privyet FNP LNP");
$this->assertEquals("Hola $a->alternatename '$a->middlename' $a->lastname", "Hola AN 'MN' LN");
}
}

0 comments on commit 699daaf

Please sign in to comment.