From 699daafc64a17ab44d2c711446d4284eab69da8b Mon Sep 17 00:00:00 2001 From: meirzamoodle Date: Thu, 11 Jul 2024 07:57:33 +0700 Subject: [PATCH] MDL-64148 core: Make email greetings consistent 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". --- .upgradenotes/MDL-64148-2024071508585319.yml | 6 +++ .../manual/tests/behat/welcomemessage.feature | 6 +-- enrol/manual/tests/lib_test.php | 2 +- enrol/self/tests/behat/welcomemessage.feature | 6 +-- enrol/tests/enrollib_test.php | 41 +++++++++++++++++++ lang/en/admin.php | 4 +- lang/en/auth.php | 2 +- lang/en/enrol.php | 2 +- lang/en/moodle.php | 6 +-- lib/classes/user.php | 16 ++++++++ lib/enrollib.php | 6 ++- lib/moodlelib.php | 22 +++++++--- lib/tests/authlib_test.php | 18 ++++++++ lib/tests/moodlelib_test.php | 41 +++++++++++++++++++ user/edit.php | 7 +++- user/tests/userlib_test.php | 35 ++++++++++++++++ 16 files changed, 200 insertions(+), 20 deletions(-) create mode 100644 .upgradenotes/MDL-64148-2024071508585319.yml diff --git a/.upgradenotes/MDL-64148-2024071508585319.yml b/.upgradenotes/MDL-64148-2024071508585319.yml new file mode 100644 index 00000000000..55d7346f6b4 --- /dev/null +++ b/.upgradenotes/MDL-64148-2024071508585319.yml @@ -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 diff --git a/enrol/manual/tests/behat/welcomemessage.feature b/enrol/manual/tests/behat/welcomemessage.feature index 6beda675906..a5434619bf7 100644 --- a/enrol/manual/tests/behat/welcomemessage.feature +++ b/enrol/manual/tests/behat/welcomemessage.feature @@ -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" @@ -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 diff --git a/enrol/manual/tests/lib_test.php b/enrol/manual/tests/lib_test.php index 567e5c246dd..576ac36e4c1 100644 --- a/enrol/manual/tests/lib_test.php +++ b/enrol/manual/tests/lib_test.php @@ -843,7 +843,7 @@ class lib_test extends \advanced_testcase { get_string( 'customwelcomemessageplaceholder', 'core_enrol', - ['fullname' => fullname($student), 'coursename' => $course->fullname], + ['firstname' => $student->firstname, 'coursename' => $course->fullname], ), $message->fullmessage, ); diff --git a/enrol/self/tests/behat/welcomemessage.feature b/enrol/self/tests/behat/welcomemessage.feature index adf2e9f5612..56fe810c1cc 100644 --- a/enrol/self/tests/behat/welcomemessage.feature +++ b/enrol/self/tests/behat/welcomemessage.feature @@ -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" @@ -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 @@ -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 diff --git a/enrol/tests/enrollib_test.php b/enrol/tests/enrollib_test.php index 654d4fe1e4b..6d5f90c7389 100644 --- a/enrol/tests/enrollib_test.php +++ b/enrol/tests/enrollib_test.php @@ -1827,4 +1827,45 @@ class enrollib_test extends advanced_testcase { $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)); + } } diff --git a/lang/en/admin.php b/lang/en/admin.php index db918e07cbd..47fe143a29c 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -815,7 +815,9 @@ $string['locationsettings'] = 'Location settings'; $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 diff --git a/lang/en/auth.php b/lang/en/auth.php index 0098a307411..3f879bee3c3 100644 --- a/lang/en/auth.php +++ b/lang/en/auth.php @@ -77,7 +77,7 @@ $string['emailchangecancel'] = 'Cancel email change'; $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: diff --git a/lang/en/enrol.php b/lang/en/enrol.php index 4b19f85ca20..820a75e5ad1 100644 --- a/lang/en/enrol.php +++ b/lang/en/enrol.php @@ -45,7 +45,7 @@ $string['customwelcomemessage_help'] = 'Accepted formats: Plain text or Moodle-a
  • User last name {$a->lastname}
  • User course role {$a->courserole}
    • '; -$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. diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 11600825284..a3f1ed63321 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -643,7 +643,7 @@ $string['emailalreadysent'] = 'A password reset email has already been sent. Ple $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. @@ -2501,8 +2501,6 @@ $string['coursesearch_help'] = 'You can search for multiple words at once and ca // 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 @@ -2522,3 +2520,5 @@ line at the top of your web browser window. 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'; diff --git a/lib/classes/user.php b/lib/classes/user.php index c411d5f86ab..1f1a0be23ee 100644 --- a/lib/classes/user.php +++ b/lib/classes/user.php @@ -1589,4 +1589,20 @@ class core_user { ]; } + /** + * 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; + } } diff --git a/lib/enrollib.php b/lib/enrollib.php index 7b2cac48c1c..fa97842e660 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -3670,7 +3670,11 @@ abstract class enrol_plugin { '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 = [ diff --git a/lib/moodlelib.php b/lib/moodlelib.php index ba6415a77d7..781c33e83f8 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -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; @@ -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)); @@ -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; @@ -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(); diff --git a/lib/tests/authlib_test.php b/lib/tests/authlib_test.php index 8a053ed6cf0..c6be2bce21a 100644 --- a/lib/tests/authlib_test.php +++ b/lib/tests/authlib_test.php @@ -549,4 +549,22 @@ class authlib_test extends \advanced_testcase { $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)); + } + } diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index 750c8a3f6b7..b3b89439f4a 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -5593,4 +5593,45 @@ EOT; ], ]; } + + /** + * 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], + ]; + } } diff --git a/user/edit.php b/user/edit.php index 0524632a7c9..0f1e0f94223 100644 --- a/user/edit.php +++ b/user/edit.php @@ -263,7 +263,12 @@ if ($userform->is_cancelled()) { $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); diff --git a/user/tests/userlib_test.php b/user/tests/userlib_test.php index 0cf16b679f7..181f19d1a89 100644 --- a/user/tests/userlib_test.php +++ b/user/tests/userlib_test.php @@ -1048,4 +1048,39 @@ class userlib_test extends \advanced_testcase { $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"); + } }