From 5b1a30237447171b42b149add6e4d39ab03110f4 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 14 Mar 2025 16:05:20 +0000 Subject: [PATCH 1/4] MDL-84733 assign: add tests of existing message behaviour This is to prove the changes in the next two commits don't change the behaviour. Note, I tidied up the whitespace in some HTML lang strings. --- mod/assign/lang/en/assign.php | 18 +++++---------- mod/assign/tests/generator.php | 2 +- mod/assign/tests/locallib_test.php | 37 ++++++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php index 388f0efa21e..f42554e10cf 100644 --- a/mod/assign/lang/en/assign.php +++ b/mod/assign/lang/en/assign.php @@ -92,8 +92,7 @@ assignment submission for \'{$a->assignment}\' You can see it appended to your assignment submission: {$a->url}'; -$string['assignmentmailhtml'] = '

{$a->grader} has posted some feedback on your -assignment submission for \'{$a->assignment}\'.

+$string['assignmentmailhtml'] = '

{$a->grader} has posted some feedback on your assignment submission for \'{$a->assignment}\'.

You can see it appended to your assignment submission.

'; $string['assignmentmailsmall'] = '{$a->grader} has posted some feedback on your assignment submission for \'{$a->assignment}\' You can see it appended to your submission'; @@ -268,8 +267,7 @@ for \'{$a->assignment}\' at {$a->timeupdated} It is available here: {$a->url}'; -$string['gradersubmissionupdatedhtml'] = '{$a->username} has updated their assignment submission -for \'{$a->assignment}\' at {$a->timeupdated}

+$string['gradersubmissionupdatedhtml'] = '{$a->username} has updated their assignment submission for \'{$a->assignment}\' at {$a->timeupdated}

It is available on the web site.'; $string['gradersubmissionupdatedsmall'] = '{$a->username} has updated their submission for assignment {$a->assignment}.'; $string['gradeuser'] = 'Grade {$a}'; @@ -286,8 +284,7 @@ assignment submission for \'{$a->assignment}\' You can see it appended to your assignment submission: {$a->url}'; -$string['feedbackavailablehtml'] = '{$a->username} has posted some feedback on your -assignment submission for \'{$a->assignment}\'

+$string['feedbackavailablehtml'] = '{$a->username} has posted some feedback on your assignment submission for \'{$a->assignment}\'

You can see it appended to your assignment submission.'; $string['feedbackavailablesmall'] = '{$a->username} has given feedback for assignment {$a->assignment}'; $string['feedbackavailableanontext'] = 'You have new feedback on your @@ -296,8 +293,7 @@ assignment submission for \'{$a->assignment}\' You can see it appended to your assignment submission: {$a->url}'; -$string['feedbackavailableanonhtml'] = 'You have new feedback on your -assignment submission for \'{$a->assignment}\'

+$string['feedbackavailableanonhtml'] = 'You have new feedback on your assignment submission for \'{$a->assignment}\'

You can see it appended to your assignment submission.'; $string['feedbackavailableanonsmall'] = 'New feedback for assignment {$a->assignment}'; $string['feedbackplugins'] = 'Feedback plugins'; @@ -580,8 +576,7 @@ $string['submissioncopiedtext'] = 'You have made a copy of your previous assignm You can see the status of your assignment submission: {$a->url}'; -$string['submissioncopiedhtml'] = '

You have made a copy of your previous -assignment submission for \'{$a->assignment}\'.

+$string['submissioncopiedhtml'] = '

You have made a copy of your previous assignment submission for \'{$a->assignment}\'.

You can see the status of your assignment submission.

'; $string['submissioncopiedsmall'] = 'You have copied your previous assignment submission for {$a->assignment}'; $string['submissiondrafts'] = 'Require students to click the submit button'; @@ -603,8 +598,7 @@ $string['submissionreceiptothertext'] = 'Your assignment submission for \'{$a->a You can see the status of your assignment submission: {$a->url}'; -$string['submissionreceiptotherhtml'] = 'Your assignment submission for -\'{$a->assignment}\' has been submitted.

+$string['submissionreceiptotherhtml'] = 'Your assignment submission for \'{$a->assignment}\' has been submitted.

You can see the status of your assignment submission.'; $string['submissionreceiptothersmall'] = 'Your assignment submission for {$a->assignment} has been submitted.'; $string['submissionreceipts'] = 'Send submission receipts'; diff --git a/mod/assign/tests/generator.php b/mod/assign/tests/generator.php index f4cbcef10a6..59ffc3b0321 100644 --- a/mod/assign/tests/generator.php +++ b/mod/assign/tests/generator.php @@ -41,7 +41,7 @@ trait mod_assign_test_generator { * Convenience function to create a testable instance of an assignment. * * @param array $params Array of parameters to pass to the generator - * @return testable_assign Testable wrapper around the assign class. + * @return mod_assign_testable_assign Testable wrapper around the assign class. */ protected function create_instance($course, $params = [], $options = []) { $params['course'] = $course->id; diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php index 5ca19a3933e..f6b4c5deae9 100644 --- a/mod/assign/tests/locallib_test.php +++ b/mod/assign/tests/locallib_test.php @@ -1765,8 +1765,41 @@ final class locallib_test extends \advanced_testcase { $messages = $sink->get_messages(); $this->assertEquals(1, count($messages)); - $this->assertEquals(1, $messages[0]->notification); - $this->assertEquals($assign->get_instance()->name, $messages[0]->contexturlname); + + // Get some bits we will need to verify the content of the message. + $assignname = $assign->get_instance()->name; + $assignurl = (new \moodle_url('/mod/assign/view.php', ['id' => $assign->get_course_module()->id]))->out(); + $teachername = fullname($teacher); + $assignsurl = (new \moodle_url('/mod/assign/index.php', ['id' => $course->id]))->out(); + $courseurl = (new \moodle_url('/course/view.php', ['id' => $course->id]))->out(); + + $message = $messages[0]; + $this->assertEquals(1, $message->notification); + $this->assertEquals($assign->get_instance()->name, $message->contexturlname); + $this->assertEquals("$teachername has given feedback for assignment $assignname", $message->subject, ); + $this->assertEquals("$course->shortname -> Assignment -> $assignname +--------------------------------------------------------------------- +$teachername has posted some feedback on your +assignment submission for '$assignname' + +You can see it appended to your assignment submission: + + $assignurl + +--------------------------------------------------------------------- +", + $message->fullmessage, + ); + $this->assertEquals( + '

' . $course->shortname . ' ' . + '->Assignment ' . + '->' . $assignname . '

' . + '

' . $teachername . ' has posted some feedback on your ' . + 'assignment submission for \'' . $assignname . '\'

' . "\n" . + 'You can see it appended to your assignment submission.

', + $message->fullmessagehtml + ); } public function test_cron_message_includes_courseid(): void { From b9bd43ad7822d0476164a99092f10fe5c87105bd Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 14 Mar 2025 14:15:02 +0000 Subject: [PATCH 2/4] MDL-84733 assignment: more information for message lang strings --- .upgradenotes/MDL-84733-2025031414225824.yml | 9 ++++++ ...ge_manage_notification_preferences.feature | 8 ++--- mod/assign/locallib.php | 32 ++++++++++++++++--- 3 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 .upgradenotes/MDL-84733-2025031414225824.yml diff --git a/.upgradenotes/MDL-84733-2025031414225824.yml b/.upgradenotes/MDL-84733-2025031414225824.yml new file mode 100644 index 00000000000..d2f598869ef --- /dev/null +++ b/.upgradenotes/MDL-84733-2025031414225824.yml @@ -0,0 +1,9 @@ +issueNumber: MDL-84733 +notes: + mod_assign: + - message: >- + When the assignment activity sends notifications, it now sets more + information in the $a object passed to the languages strings for the + message subject and body. This is to give more flexibility to people + using Language Customisation. + type: improved diff --git a/message/tests/behat/message_manage_notification_preferences.feature b/message/tests/behat/message_manage_notification_preferences.feature index 54107c84b9d..c833249a305 100644 --- a/message/tests/behat/message_manage_notification_preferences.feature +++ b/message/tests/behat/message_manage_notification_preferences.feature @@ -143,14 +143,14 @@ Feature: Manage notification preferences - Email # Open the popover. And I open the notification popover # Confirm the submission notification is NOT visible. - And I should not see "You have submitted your assignment submission for Test assignment name" in the "#nav-notification-popover-container" "css_element" + And I should not see "Assignment Submission Confirmation" in the "#nav-notification-popover-container" "css_element" And I log in as "student2" # Confirm the popover is showing the unread notifications. Then I should see "1" in the "#nav-notification-popover-container [data-region='count-container']" "css_element" # Open the popover. And I open the notification popover # Confirm the submission notification is visible. - And I should see "Assignment Submission Confirmation - Test assignment name" in the "#nav-notification-popover-container" "css_element" + And I should see "Assignment Submission Confirmation" in the "#nav-notification-popover-container" "css_element" Scenario: User cannot disable forced notification preferences Given the following "courses" exist: @@ -182,7 +182,7 @@ Feature: Manage notification preferences - Email # Open the popover. And I open the notification popover # Confirm the submission notification is visible. - And I should see "Assignment Submission Confirmation - Test assignment name" in the "#nav-notification-popover-container" "css_element" + And I should see "Assignment Submission Confirmation" in the "#nav-notification-popover-container" "css_element" Scenario: User cannot disable disallowed notification preferences Given the following "courses" exist: @@ -214,7 +214,7 @@ Feature: Manage notification preferences - Email # Open the popover. And I open the notification popover # Confirm the submission notification is NOT visible. - And I should not see "Assignment Submission Confirmation - Test assignment name" in the "#nav-notification-popover-container" "css_element" + And I should not see "Assignment Submission Confirmation" in the "#nav-notification-popover-container" "css_element" Scenario: Toggle notification preferences hides/displays options Given I log in as "admin" diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 2bc447420f4..c7061fd0a6a 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -6506,7 +6506,7 @@ class assign { * @param string $eventtype * @param int $updatetime * @param stdClass $coursemodule - * @param stdClass $context + * @param context $context * @param stdClass $course * @param string $modulename * @param string $assignmentname @@ -6530,6 +6530,11 @@ class assign { $extrainfo = []) { global $CFG, $PAGE; + // We put more data into info that is used by the default language strings, + // so that there is more flexibility for organisations that want to use + // Language Customisation to customise the messages. + + // Information about the sender - normally the user who performed the action. $info = new stdClass(); if ($blindmarking) { $userfrom = clone($userfrom); @@ -6538,14 +6543,33 @@ class assign { $userfrom->lastname = $uniqueidforuser; $userfrom->email = $CFG->noreplyaddress; } else { - $info->username = fullname($userfrom, true); + $info->username = core_user::get_fullname($userfrom, $context, ['override' => true]); } - $info->assignment = format_string($assignmentname, true, array('context'=>$context)); - $info->url = $CFG->wwwroot.'/mod/assign/view.php?id='.$coursemodule->id; + + // Information about the recipient (for greeting, etc). + $info->recipentname = core_user::get_fullname($userto, $context, ['override' => true]); + + // Information about the assignment. + $info->assignment = format_string($assignmentname, true, ['context' => $context]); + $info->url = $CFG->wwwroot . '/mod/assign/view.php?id=' . $coursemodule->id; + $info->assignmentlink = '' . s($info->assignment) . ' report'; + $info->assigncmid = $coursemodule->id; + + // Information about the course. + $info->courseshortname = format_string($course->shortname, true, ['context' => $context]); + $info->coursefullname = format_string($course->fullname, true, ['context' => $context]); + $info->courseurl = $CFG->wwwroot . '/course/view.php?id=' . $course->id; + $info->courseassignsurl = $CFG->wwwroot . '/mod/assign/index.php?id=' . $course->id; + + // Time of the action. $info->timeupdated = userdate($updatetime, get_string('strftimerecentfull')); + + // Other data passed in. $info = (object) array_merge((array) $info, $extrainfo); + // Prepare the message subject and body. $postsubject = get_string($messagetype . 'small', 'assign', $info); + $posttext = self::format_notification_message_text($messagetype, $info, $course, From aa2fb1e3d2abd425651a2dc6b6c3edc3300f7ed9 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 14 Mar 2025 14:48:05 +0000 Subject: [PATCH 3/4] MDL-84733 assign: use a template to render message bodies The overall structure of these messages, which adds the course and assignment name to whatever comes from the particular message language string, was hard-coded in the assignment code. By changing this to use a template, now themes can override this. --- .upgradenotes/MDL-84733-2025031414225824.yml | 11 ++- mod/assign/locallib.php | 93 +++---------------- .../messages/notification_html.mustache | 63 +++++++++++++ .../messages/notification_text.mustache | 60 ++++++++++++ mod/assign/tests/locallib_test.php | 18 ++-- 5 files changed, 158 insertions(+), 87 deletions(-) create mode 100644 mod/assign/templates/messages/notification_html.mustache create mode 100644 mod/assign/templates/messages/notification_text.mustache diff --git a/.upgradenotes/MDL-84733-2025031414225824.yml b/.upgradenotes/MDL-84733-2025031414225824.yml index d2f598869ef..e1139e1076e 100644 --- a/.upgradenotes/MDL-84733-2025031414225824.yml +++ b/.upgradenotes/MDL-84733-2025031414225824.yml @@ -5,5 +5,14 @@ notes: When the assignment activity sends notifications, it now sets more information in the $a object passed to the languages strings for the message subject and body. This is to give more flexibility to people - using Language Customisation. + using Language Customisation. The avaiable information is the same as + the template context for the two templates in the next paragraph, but + without the messagetext/html. + + Also, the overall structure of these messages is now rendered using + templates mod_assign/messages/notification_html and + mod_assign/messages/notification_html, so they can be overridden by themes. + As a result of this, the methods format_notification_message_text and + format_notification_message_html (which should have been private to + mod_assign and not used anywhere else) have been removed. type: improved diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index c7061fd0a6a..704b7df7311 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -6433,70 +6433,6 @@ class assign { return $notifiableusers; } - /** - * Format a notification for plain text. - * - * @param string $messagetype - * @param stdClass $info - * @param stdClass $course - * @param stdClass $context - * @param string $modulename - * @param string $assignmentname - */ - protected static function format_notification_message_text($messagetype, - $info, - $course, - $context, - $modulename, - $assignmentname) { - $formatparams = array('context' => $context->get_course_context()); - $posttext = format_string($course->shortname, true, $formatparams) . - ' -> ' . - $modulename . - ' -> ' . - format_string($assignmentname, true, $formatparams) . "\n"; - $posttext .= '---------------------------------------------------------------------' . "\n"; - $posttext .= get_string($messagetype . 'text', 'assign', $info)."\n"; - $posttext .= "\n---------------------------------------------------------------------\n"; - return $posttext; - } - - /** - * Format a notification for HTML. - * - * @param string $messagetype - * @param stdClass $info - * @param stdClass $course - * @param stdClass $context - * @param string $modulename - * @param stdClass $coursemodule - * @param string $assignmentname - */ - protected static function format_notification_message_html($messagetype, - $info, - $course, - $context, - $modulename, - $coursemodule, - $assignmentname) { - global $CFG; - $formatparams = array('context' => $context->get_course_context()); - $posthtml = '

' . - '' . - format_string($course->shortname, true, $formatparams) . - ' ->' . - '' . - $modulename . - ' ->' . - '' . - format_string($assignmentname, true, $formatparams) . - '

'; - $posthtml .= '
'; - $posthtml .= '

' . get_string($messagetype . 'html', 'assign', $info) . '

'; - $posthtml .= '
'; - return $posthtml; - } - /** * Message someone about something (static so it can be called from cron). * @@ -6508,7 +6444,7 @@ class assign { * @param stdClass $coursemodule * @param context $context * @param stdClass $course - * @param string $modulename + * @param string $modulename - no longer used. * @param string $assignmentname * @param bool $blindmarking * @param int $uniqueidforuser @@ -6567,26 +6503,25 @@ class assign { // Other data passed in. $info = (object) array_merge((array) $info, $extrainfo); - // Prepare the message subject and body. + // Prepare the message subject and bodies. $postsubject = get_string($messagetype . 'small', 'assign', $info); - $posttext = self::format_notification_message_text($messagetype, - $info, - $course, - $context, - $modulename, - $assignmentname); + $renderer = $PAGE->get_renderer('mod_assign'); + $context = clone $info; + $context->messagetext = get_string($messagetype . 'text', 'assign', $info); + // Mustache strips off all training whitespace, but we want a newline at the end. + $posttext = $renderer->render_from_template( + 'mod_assign/messages/notification_text', $context) . "\n"; + $posthtml = ''; if ($userto->mailformat == 1) { - $posthtml = self::format_notification_message_html($messagetype, - $info, - $course, - $context, - $modulename, - $coursemodule, - $assignmentname); + $context = clone $info; + $context->messagehtml = get_string($messagetype . 'html', 'assign', $info); + $posthtml = $renderer->render_from_template( + 'mod_assign/messages/notification_html', $context); } + // Build the message object. $eventdata = new \core\message\message(); $eventdata->courseid = $course->id; $eventdata->modulename = 'assign'; diff --git a/mod/assign/templates/messages/notification_html.mustache b/mod/assign/templates/messages/notification_html.mustache new file mode 100644 index 00000000000..07c1b87b701 --- /dev/null +++ b/mod/assign/templates/messages/notification_html.mustache @@ -0,0 +1,63 @@ +{{! + This file is part of Moodle - http://moodle.org/ + + Moodle is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Moodle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Moodle. If not, see . +}} +{{! + @template mod_assign/messages/notification_html + + This template is used for the HTML version of messages sent from the assignment module. + + Note: much more is passed to this template than might be needed, but this is to give + flexibility to themes. + + Classes required for JS: + * none + + Context variables required for this template: + * messagehtml - the main content of the message. Normally comes from a lang string. + * username - full name of the user who performed the action leading to the message, but will be changed to 'Participant 123' if blink marking is in use. + * recipentname - full name of the user the email is being sent to. (Never anonymised.) + * assignment - The name of this assignment. + * url - The URL of this assignment activity. + * assigncmid - Course-module id of this assignment. + * courseshortname - short name of the course this assignment is in. + * coursefullname - full name of the course this assignment is in. + * courseurl - URL of the course page. + * courseassignsurl - URL of the paeg listing all assignments in the course. + * timeupdated - Time the action that triggered this message was performed, formatted for display. + + Example context (json): + { + "messagehtml": "

Your assignment for Assignment 1 has been submitted. You can check its status on the assignment page.

", + "username": "Trusty Teacher", + "recipientname": "Smart Student", + "assignment": "Assignment 1", + "url": "https://qa.moodledemo.net/mod/assign/view.php?id=5", + "assigncmid": 5, + "courseshortname": "A100", + "coursefullname": "Assessment for beginners", + "courseurl": "https://qa.moodledemo.net/course/view.php?id=2", + "courseassignsurl": "https://qa.moodledemo.net/mod/assign/index.php?id=2", + "timeupdated": "Fri, 14 Mar 2025, 11:08 PM" + } +}} +

+ {{{courseshortname}}} -> + {{#str}}modulename, mod_assign{{/str}} -> + {{{assignment}}} +

+
+

{{{messagehtml}}}

 +
diff --git a/mod/assign/templates/messages/notification_text.mustache b/mod/assign/templates/messages/notification_text.mustache new file mode 100644 index 00000000000..7538f0fb10d --- /dev/null +++ b/mod/assign/templates/messages/notification_text.mustache @@ -0,0 +1,60 @@ +{{! + This file is part of Moodle - http://moodle.org/ + + Moodle is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Moodle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Moodle. If not, see . +}} +{{! + @template mod_assign/messages/notification_text + + This template is used for the plain text version of messages sent from the assignment module. + + Note: much more is passed to this template than might be needed, but this is to give + flexibility to themes. + + Classes required for JS: + * none + + Context variables required for this template: + * messagetext - the main content of the message. Normally comes from a lang string. + * username - full name of the user who performed the action leading to the message, but will be changed to 'Participant 123' if blink marking is in use. + * recipentname - full name of the user the email is being sent to. (Never anonymised.) + * assignment - The name of this assignment. + * url - The URL of this assignment activity. + * assigncmid - Course-module id of this assignment. + * courseshortname - short name of the course this assignment is in. + * coursefullname - full name of the course this assignment is in. + * courseurl - URL of the course page. + * courseassignsurl - URL of the paeg listing all assignments in the course. + * timeupdated - Time the action that triggered this message was performed, formatted for display. + + Example context (json): + { + "messagehtml": "Your assignment for Assignment 1 has been submitted. You can check its status at https://qa.moodledemo.net/mod/assign/view.php?id=5.", + "username": "Trusty Teacher", + "recipientname": "Smart Student", + "assignment": "Assignment 1", + "url": "https://qa.moodledemo.net/mod/assign/view.php?id=5", + "assigncmid": 5, + "courseshortname": "A100", + "coursefullname": "Assessment for beginners", + "courseurl": "https://qa.moodledemo.net/course/view.php?id=2", + "courseassignsurl": "https://qa.moodledemo.net/mod/assign/index.php?id=2", + "timeupdated": "Fri, 14 Mar 2025, 11:08 PM" + } +}} +{{{courseshortname}}} -> {{#str}}modulename, mod_assign{{/str}} -> {{{assignment}}} +--------------------------------------------------------------------- +{{{messagetext}}} + +--------------------------------------------------------------------- diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php index f6b4c5deae9..111c89cc43a 100644 --- a/mod/assign/tests/locallib_test.php +++ b/mod/assign/tests/locallib_test.php @@ -1791,13 +1791,17 @@ You can see it appended to your assignment submission: $message->fullmessage, ); $this->assertEquals( - '

' . $course->shortname . ' ' . - '->Assignment ' . - '->' . $assignname . '

' . - '

' . $teachername . ' has posted some feedback on your ' . - 'assignment submission for \'' . $assignname . '\'

' . "\n" . - 'You can see it appended to your assignment submission.

', + '

+ ' . $course->shortname . ' -> + Assignment -> + ' . $assignname . ' +

+
+

' . $teachername . ' has posted some feedback on your ' . + 'assignment submission for \'' . $assignname . '\'

+You can see it appended to your assignment submission.

 +
', $message->fullmessagehtml ); } From 3494adf5c974cfd0d19ff21c719e35327fc92598 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 21 Mar 2025 14:30:34 +0000 Subject: [PATCH 4/4] MDL-84733 assig: fix HTML special characters in emails --- mod/assign/locallib.php | 18 ++++++++++++++---- mod/assign/tests/locallib_test.php | 15 ++++++++------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 704b7df7311..2897b528868 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -6482,18 +6482,20 @@ class assign { $info->username = core_user::get_fullname($userfrom, $context, ['override' => true]); } - // Information about the recipient (for greeting, etc). + // Information about the recipient (for greeting, etc.). $info->recipentname = core_user::get_fullname($userto, $context, ['override' => true]); // Information about the assignment. $info->assignment = format_string($assignmentname, true, ['context' => $context]); $info->url = $CFG->wwwroot . '/mod/assign/view.php?id=' . $coursemodule->id; + // Note: URLs here avoid the & character to avoid escaping issues between text and HTML messages. $info->assignmentlink = '' . s($info->assignment) . ' report'; $info->assigncmid = $coursemodule->id; // Information about the course. $info->courseshortname = format_string($course->shortname, true, ['context' => $context]); $info->coursefullname = format_string($course->fullname, true, ['context' => $context]); + // Note: URLs here avoid the & character to avoid escaping issues between text and HTML messages. $info->courseurl = $CFG->wwwroot . '/course/view.php?id=' . $course->id; $info->courseassignsurl = $CFG->wwwroot . '/mod/assign/index.php?id=' . $course->id; @@ -6503,12 +6505,20 @@ class assign { // Other data passed in. $info = (object) array_merge((array) $info, $extrainfo); + // Since format_string returns text with HTML characters escaped, + // we need to un-escape before including in the plain text email and subject line. + // (Test with a course or assignment called "Escaping & unescaping" to understand). + $plaintextinfo = clone $info; + $plaintextinfo->assignment = html_entity_decode($info->assignment); + $plaintextinfo->courseshortname = html_entity_decode($info->courseshortname); + $plaintextinfo->coursefullname = html_entity_decode($info->coursefullname); + // Prepare the message subject and bodies. - $postsubject = get_string($messagetype . 'small', 'assign', $info); + $postsubject = get_string($messagetype . 'small', 'assign', $plaintextinfo); $renderer = $PAGE->get_renderer('mod_assign'); - $context = clone $info; - $context->messagetext = get_string($messagetype . 'text', 'assign', $info); + $context = clone $plaintextinfo; + $context->messagetext = get_string($messagetype . 'text', 'assign', $plaintextinfo); // Mustache strips off all training whitespace, but we want a newline at the end. $posttext = $renderer->render_from_template( 'mod_assign/messages/notification_text', $context) . "\n"; diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php index 111c89cc43a..9b850bbbdf9 100644 --- a/mod/assign/tests/locallib_test.php +++ b/mod/assign/tests/locallib_test.php @@ -1724,13 +1724,14 @@ final class locallib_test extends \advanced_testcase { \core\cron::setup_user(); \assign::cron(); - $course = $this->getDataGenerator()->create_course(); + $course = $this->getDataGenerator()->create_course(['shortname' => 'E&U']); $teacher = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher'); $student = $this->getDataGenerator()->create_and_enrol($course, 'student'); // Now create an assignment and add some feedback. $this->setUser($teacher); $assign = $this->create_instance($course, [ + 'name' => 'Escaping & Unescaping', 'sendstudentnotifications' => 1, 'markingworkflow' => 1, ]); @@ -1775,12 +1776,12 @@ final class locallib_test extends \advanced_testcase { $message = $messages[0]; $this->assertEquals(1, $message->notification); - $this->assertEquals($assign->get_instance()->name, $message->contexturlname); + $this->assertEquals(format_string($assign->get_instance()->name), $message->contexturlname); $this->assertEquals("$teachername has given feedback for assignment $assignname", $message->subject, ); - $this->assertEquals("$course->shortname -> Assignment -> $assignname + $this->assertEquals("E&U -> Assignment -> Escaping & Unescaping --------------------------------------------------------------------- $teachername has posted some feedback on your -assignment submission for '$assignname' +assignment submission for 'Escaping & Unescaping' You can see it appended to your assignment submission: @@ -1792,13 +1793,13 @@ You can see it appended to your assignment submission: ); $this->assertEquals( '

- ' . $course->shortname . ' -> + E&U -> Assignment -> - ' . $assignname . ' + Escaping & Unescaping

' . $teachername . ' has posted some feedback on your ' . - 'assignment submission for \'' . $assignname . '\'

+ 'assignment submission for \'Escaping & Unescaping\'

You can see it appended to your assignment submission.
',