From df0670b959df69fc8a079604d4bd1fa5c723a578 Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Tue, 13 Aug 2024 11:25:15 +1000 Subject: [PATCH] [#139] Ensure all emails send same message --- lib.php | 71 ++++---------- tests/session_test.php | 216 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 52 deletions(-) diff --git a/lib.php b/lib.php index ed41045..39c2b6e 100644 --- a/lib.php +++ b/lib.php @@ -2238,6 +2238,12 @@ function facetoface_send_notice($postsubject, $posttext, $posttextmgrheading, } } + // Define a simple reusable function so we don't have to copy + // and paste a huge function call multiple times. + $substitute = function($text, $session) use ($facetoface, $user) { + return facetoface_email_substitutions($text, format_string($facetoface->name), $facetoface->reminderperiod, $user, $session, $session->id); + }; + // Do iCal attachement stuff. $icalattachments = []; if ($notificationtype & MDL_F2F_ICAL) { @@ -2249,28 +2255,11 @@ function facetoface_send_notice($postsubject, $posttext, $posttextmgrheading, $session->sessiondates = [$sessiondate]; // One day at a time. $filename = facetoface_get_ical_attachment($notificationtype, $facetoface, $session, $user); - $subject = facetoface_email_substitutions( - $postsubject, - format_string($facetoface->name), - $facetoface->reminderperiod, - $user, - $session, - $session->id - ); - $body = facetoface_email_substitutions($posttext, format_string($facetoface->name), $facetoface->reminderperiod, - $user, $session, $session->id); - $htmlmessage = facetoface_email_substitutions( - $posttext, - $facetoface->name, - $facetoface->reminderperiod, - $user, - $session, - $session->id - ); - $htmlbody = $htmlmessage; + $subject = $substitute($postsubject, $session); + $body = $substitute($posttext, $session); $icalattachments[] = [ 'filename' => $filename, 'subject' => $subject, - 'body' => $body, 'htmlbody' => $htmlbody, + 'body' => $body, ]; } @@ -2278,42 +2267,20 @@ function facetoface_send_notice($postsubject, $posttext, $posttextmgrheading, $session->sessiondates = $sessiondates; } else { $filename = facetoface_get_ical_attachment($notificationtype, $facetoface, $session, $user); - $subject = facetoface_email_substitutions($postsubject, format_string($facetoface->name), $facetoface->reminderperiod, - $user, $session, $session->id); - $body = facetoface_email_substitutions($posttext, format_string($facetoface->name), $facetoface->reminderperiod, - $user, $session, $session->id); - $htmlmessage = facetoface_email_substitutions( - $posttext, - $facetoface->name, - $facetoface->reminderperiod, - $user, - $session, - $session->id - ); - $htmlbody = $htmlmessage; + $subject = $substitute($postsubject, $session); + $body = $substitute($posttext, $session); $icalattachments[] = [ 'filename' => $filename, 'subject' => $subject, - 'body' => $body, 'htmlbody' => $htmlbody, + 'body' => $body, ]; } } // Fill-in the email placeholders. - $postsubject = facetoface_email_substitutions($postsubject, format_string($facetoface->name), $facetoface->reminderperiod, - $user, $session, $session->id); - $posttext = facetoface_email_substitutions($posttext, format_string($facetoface->name), $facetoface->reminderperiod, - $user, $session, $session->id); - - $posttextmgrheading = facetoface_email_substitutions( - $posttextmgrheading, - format_string($facetoface->name), - $facetoface->reminderperiod, - $user, - $session, - $session->id - ); + $postsubject = $substitute($postsubject, $session); + $posttext = $substitute($posttext, $session); + $posttextmgrheading = $substitute($posttextmgrheading, $session); - $posthtml = ''; // FIXME. if ($fromaddress = get_config('facetoface', 'fromaddress')) { $from = new stdClass(); $from->maildisplay = true; @@ -2326,7 +2293,7 @@ function facetoface_send_notice($postsubject, $posttext, $posttextmgrheading, if ($notificationtype & MDL_F2F_ICAL) { foreach ($icalattachments as $attachment) { if (!email_to_user($user, $from, $attachment['subject'], $attachment['body'], - $attachment['htmlbody'], $attachment['filename'], $attachmentfilename)) { + '', $attachment['filename'], $attachmentfilename)) { return 'error:cannotsendconfirmationuser'; } unlink($CFG->dataroot . '/' . $attachment['filename']); @@ -2335,7 +2302,7 @@ function facetoface_send_notice($postsubject, $posttext, $posttextmgrheading, // Send plain text email. if ($notificationtype & MDL_F2F_TEXT - && !email_to_user($user, $from, $postsubject, $posttext, $posthtml)) { + && !email_to_user($user, $from, $postsubject, $posttext)) { return 'error:cannotsendconfirmationuser'; } @@ -2347,7 +2314,7 @@ function facetoface_send_notice($postsubject, $posttext, $posttextmgrheading, $manager->email = $manageremail; // Leave out the ical attachments in the managers notification. - if (!email_to_user($manager, $from, $postsubject, $managertext, $posthtml)) { + if (!email_to_user($manager, $from, $postsubject, $managertext)) { return 'error:cannotsendconfirmationmanager'; } } @@ -2361,7 +2328,7 @@ function facetoface_send_notice($postsubject, $posttext, $posttextmgrheading, $thirdparty->email = trim($recipient); // Leave out the ical attachments in the 3rd parties notification. - if (!email_to_user($thirdparty, $from, $postsubject, $posttext, $posthtml)) { + if (!email_to_user($thirdparty, $from, $postsubject, $posttext)) { return 'error:cannotsendconfirmationthirdparty'; } } diff --git a/tests/session_test.php b/tests/session_test.php index 83ba463..eb97d80 100644 --- a/tests/session_test.php +++ b/tests/session_test.php @@ -18,6 +18,10 @@ use core_date; +defined('MOODLE_INTERNAL') || die(); +global $CFG; +require_once("$CFG->dirroot/mod/facetoface/lib.php"); + /** * Test the session helper class. * @@ -112,4 +116,216 @@ public function test_get_readable_session_time_with_users_timezone() { $expectedstring = "1 January 2030, 9:00 AM - 4 January 2030, 5:00 PM (time zone: $expectedtimezone)"; $this->assertEquals($expectedstring, session::get_readable_session_datetime($date)); } + + /** + * Provides values to test_email_notification + * @return array + */ + public static function email_notification_provider(): array { + $htmlconfirmmessage = " +

This is the confirm message

+

Details:

+ [details] + "; + + $htmldetails = " +

This is a html message

+
+ + "; + + $expectedhtmlmessage = "

+

This is the confirm message

Details:

This is a html message
+
+ * Test1
+ * Test2
+
+
+
"; + + // Because moodle code standards specify spaces over tabs, editors will automatically insert spaces + // into the string above instead of tabs. + // However we need tabs there, because this is what html_to_text uses for converting
  • to lists with a '\t*'. + $expectedhtmlmessage = str_replace(' *', "\t*", $expectedhtmlmessage); + + $plaintextmessage = 'This is a plain text message + It has plain text stuff in it + * This is a fake list + * Another fake list item + (test)[test]{test}!!@@##//// + [details]'; + + $plaintextdetails = "This is a plain text detail + It has plain text stuff in it"; + + $expectedplaintextmessage = "This is a plain text message
    + It has plain text stuff in it
    + * This is a fake list
    + * Another fake list item
    + (test)[test]{test}!!@@##////
    + This is a plain text detail
    +It has plain text stuff in it"; + + // phpcs:ignore. + $expectedhtmlandplainmessage = "

    This is the confirm message

    Details:

    This is a plain text detail
    +It has plain text stuff in it
    "; + + // Generate a matrix of tests, with all the types, and all the message combinations. + $types = [ + 'both' => [ + 'type' => MDL_F2F_BOTH, + 'emails' => 2, + 'icalemails' => 1, + ], + 'ical only' => [ + 'type' => MDL_F2F_ICAL, + 'emails' => 1, + 'icalemails' => 1, + ], + 'text only' => [ + 'type' => MDL_F2F_TEXT, + 'emails' => 1, + 'icalemails' => 0, + ], + ]; + $messages = [ + 'html message and html details' => [ + 'message' => $htmlconfirmmessage, + 'details' => $htmldetails, + 'expected' => $expectedhtmlmessage, + ], + 'plain message and plain details' => [ + 'message' => $plaintextmessage, + 'details' => $plaintextdetails, + 'expected' => $expectedplaintextmessage, + ], + 'plain text message and html details' => [ + 'message' => $htmlconfirmmessage, + 'details' => $plaintextdetails, + 'expected' => $expectedhtmlandplainmessage, + ], + ]; + + $tests = []; + foreach ($types as $typename => $typedata) { + foreach ($messages as $messagename => $messagedata) { + $testname = 'email with message: ' . $messagename . ' with type: ' . $typename; + + $tests[$testname] = [ + 'type' => $typedata['type'], + 'confirmmessage' => $messagedata['message'], + 'details' => $messagedata['details'], + 'expectedcount' => $typedata['emails'], + 'expectedicalamount' => $typedata['icalemails'], + 'expectedmessage' => $messagedata['expected'], + ]; + } + } + + return $tests; + } + + /** + * Tests email notification construction. + * @param int $notifytype type of notification + * @param string $confirmmessage the confirmation message to set + * @param string $details details to set in f2f settings + * @param int $expectedemailcount expected amount of emails that should be sent + * @param int $expectedicalamount expected amount of email with ical attachments that should be sent + * @param string $expectedmessage a string that the output of all emails should contain + * @dataProvider email_notification_provider + */ + public function test_email_notification(int $notifytype, string $confirmmessage, string $details, int $expectedemailcount, + int $expectedicalamount, string $expectedmessage) { + + /** @var \mod_facetoface_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('mod_facetoface'); + + // Setup course, f2f and user. + $course = $this->getDataGenerator()->create_course(); + $facetoface = $generator->create_instance([ + 'course' => $course->id, + 'confirmationsubject' => 'Confirmation', + 'confirmationmessage' => $confirmmessage, + 'confirmationmessageformat' => FORMAT_HTML, + ]); + $user = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->create_and_enrol($course, 'student'); + + // Create a session with start and end times. + $now = time(); + $session = $generator->create_session([ + 'facetoface' => $facetoface->id, + 'capacity' => '3', + 'allowoverbook' => '0', + 'details' => $details, + 'duration' => '1.5', // One and half hours. + 'normalcost' => '111', + 'discountcost' => '11', + 'allowcancellations' => '0', + 'sessiondates' => [ + ['timestart' => $now + 1 * DAYSECS, 'timefinish' => $now + 2 * DAYSECS], + ], + ]); + + // Sign user up to session, capturing emails. + $sink = $this->redirectEmails(); + facetoface_user_signup($session, $facetoface, $course, '', $notifytype, MDL_F2F_STATUS_BOOKED, $user->id); + $messages = $sink->get_messages(); + $this->assertCount($expectedemailcount, $messages); + + // Ensure number of ical attachment emails is same as expected. + $icalemails = array_filter($messages, function($message) { + return str_contains($message->body, 'Content-Disposition: attachment; filename=invite.ics'); + }); + $this->assertCount($expectedicalamount, $icalemails); + + // Do a very crude form of email multi-mime message parsing. + // to extract the plaintext and html segments of the email. + $messagessections = array_map(function($message) { + // Split on '--' which is the start of the separator in the email html multi-mime message. + $sections = explode('--', $message->body); + + // Extract the html section. + $htmlsection = current(array_filter($sections, function($section) { + return str_contains($section, 'text/html'); + })); + $htmllines = explode("\n", $htmlsection); + unset($htmllines[0]); + $html = implode("\n", $htmllines); + $html = quoted_printable_decode($html); + + // Do the same for the plaintext. + $plaintextsection = current(array_filter($sections, function($section) { + return str_contains($section, 'text/plain'); + })); + $plaintextlines = explode("\n", $plaintextsection); + unset($plaintextlines[0]); + $plaintext = implode("\n", $plaintextlines); + $plaintext = quoted_printable_decode($plaintext); + + return [ + 'html' => $html, + 'plaintext' => $plaintext, + ]; + }, $messages); + + $messagehtmls = array_column($messagessections, 'html'); + $messageplaintexts = array_column($messagessections, 'plaintext'); + + // Ensure each message has both html and plaintext. + $this->assertTrue(count($messagehtmls) == count($messageplaintexts)); + + // Ensure all the HTML messages are the same + // (note this is only applicable for 'both' because it's the only one that sends two emails). + $this->assertCount(1, array_unique($messagehtmls), "Emails should have the same HTML message"); + + // Ensure each message has the expected html. + foreach ($messagehtmls as $messagehtml) { + $this->assertStringContainsString($expectedmessage, $messagehtml); + } + } }