Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#139] Ensure all emails send same message #177

Merged
merged 2 commits into from
Aug 23, 2024
Merged

[#139] Ensure all emails send same message #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa4dca4
[#139] Ensure all emails send same message
matthewhilton Aug 13, 2024
0291d86
chore: php version compatibility
matthewhilton Aug 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion classes/booking_manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ public function validate($timenow = null): array {
global $DB;
$errors = [];
$sessioncapacitycache = [];
$timenow ??= time();

if ($timenow == null) {
$timenow = time();
}

// Break into rows and validate the multiple interdependant fields together.
foreach ($this->get_iterator() as $index => $entry) {
Expand Down
77 changes: 24 additions & 53 deletions lib.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,7 +1123,11 @@ function facetoface_get_future_sessions(int $id): array {
$now = time();
return array_filter(
facetoface_get_sessions($id),
fn (stdClass $s): bool => !empty(array_filter($s->sessiondates, fn (stdClass $d): bool => $d->timestart > $now))
function(stdClass $s) use ($now) {
return !empty(array_filter($s->sessiondates, function(stdClass $d) use ($now) {
return $d->timestart > $now;
}));
}
);
}

Expand Down Expand Up @@ -2238,6 +2242,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) {
Expand All @@ -2249,71 +2259,32 @@ 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,
];
}

// Restore session dates.
$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;
Expand All @@ -2326,7 +2297,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']);
Expand All @@ -2335,7 +2306,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';
}

Expand All @@ -2347,7 +2318,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';
}
}
Expand All @@ -2361,7 +2332,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';
}
}
Expand Down
216 changes: 216 additions & 0 deletions tests/session_test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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 = "
<p> This is the confirm message </p>
<p> Details: </p>
[details]
";

$htmldetails = "
<p> This is a html message </p>
<br />
<ul>
<li> Test1 </li>
<li> Test2 </li>
</ul>
";

$expectedhtmlmessage = "<div class=\"text_to_html\"><br />
<p> This is the confirm message </p><p> Details: </p> This is a html message<br />
<br />
* Test1<br />
* Test2<br />
<br />
<br />
</div>";

// 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 <li> 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<br />
It has plain text stuff in it<br />
* This is a fake list<br />
* Another fake list item<br />
(test)[test]{test}!!@@##////<br />
This is a plain text detail<br />
It has plain text stuff in it";

// phpcs:ignore.
$expectedhtmlandplainmessage = "<p> This is the confirm message </p><p> Details: </p> This is a plain text detail<br />
It has plain text stuff in it<br />";

// 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 strpos($message->body, 'Content-Disposition: attachment; filename=invite.ics') != false;
});
$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 strpos($section, 'text/html') != false;
}));
$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 strpos($section, 'text/plain') != false;
}));
$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);
}
}
}
Loading