diff --git a/badges/tests/badgeslib_test.php b/badges/tests/badgeslib_test.php index a5715695899..1ed3a1b8443 100644 --- a/badges/tests/badgeslib_test.php +++ b/badges/tests/badgeslib_test.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; require_once($CFG->libdir . '/badgeslib.php'); -class core_badgeslib_testcase extends advanced_testcase { +class core_badges_badgeslib_testcase extends advanced_testcase { protected $badgeid; protected $course; protected $user; @@ -41,6 +41,8 @@ class core_badgeslib_testcase extends advanced_testcase { global $DB, $CFG; $this->resetAfterTest(true); + unset_config('noemailever'); + $user = $this->getDataGenerator()->create_user(); $fordb = new stdClass(); @@ -239,7 +241,10 @@ class core_badgeslib_testcase extends advanced_testcase { $current = $c->get_data($activities[$this->module->cmid], false, $this->user->id); $current->completionstate = COMPLETION_COMPLETE; $current->timemodified = time(); + $sink = $this->redirectEmails(); $c->internal_set_data($activities[$this->module->cmid], $current); + $this->assertCount(1, $sink->get_messages()); + $sink->close(); // Check if badge is awarded. $this->assertDebuggingCalled('Error baking badge image!'); @@ -261,7 +266,10 @@ class core_badgeslib_testcase extends advanced_testcase { $ccompletion = new completion_completion(array('course' => $this->course->id, 'userid' => $this->user->id)); // Mark course as complete. + $sink = $this->redirectEmails(); $ccompletion->mark_complete(); + $this->assertCount(1, $sink->get_messages()); + $sink->close(); // Check if badge is awarded. $this->assertDebuggingCalled('Error baking badge image!'); @@ -281,7 +289,10 @@ class core_badgeslib_testcase extends advanced_testcase { $criteria_overall1->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ALL, 'field_address' => 'address')); $this->user->address = 'Test address'; + $sink = $this->redirectEmails(); user_update_user($this->user, false); + $this->assertCount(1, $sink->get_messages()); + $sink->close(); // Check if badge is awarded. $this->assertDebuggingCalled('Error baking badge image!'); $this->assertTrue($badge->is_issued($this->user->id)); @@ -300,7 +311,10 @@ class core_badgeslib_testcase extends advanced_testcase { $criteria_overall1->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ALL, 'field_address' => 'address')); $this->user->address = 'Test address'; + $sink = $this->redirectEmails(); user_update_user($this->user, false); + $this->assertCount(1, $sink->get_messages()); + $sink->close(); // Check if badge is awarded. $this->assertDebuggingCalled('Error baking badge image!'); $awards = $badge->get_awards(); diff --git a/course/tests/courserequest_test.php b/course/tests/courserequest_test.php index 0b23a4eabb8..3b85c334ae6 100644 --- a/course/tests/courserequest_test.php +++ b/course/tests/courserequest_test.php @@ -81,7 +81,8 @@ class core_course_courserequest_testcase extends advanced_testcase { $this->resetAfterTest(true); $this->preventResetByRollback(); - $this->setAdminUser(); + unset_config('noemailever'); + $defaultcategory = $DB->get_field_select('course_categories', "MIN(id)", "parent=0"); set_config('enablecourserequests', 1); set_config('requestcategoryselection', 0); @@ -91,6 +92,8 @@ class core_course_courserequest_testcase extends advanced_testcase { $cat1 = $this->getDataGenerator()->create_category(); $cat2 = $this->getDataGenerator()->create_category(); + $requester = $this->getDataGenerator()->create_user(); + $data = new stdClass(); $data->fullname = 'Həllo World!'; $data->shortname = 'Hi th€re!'; @@ -99,9 +102,13 @@ class core_course_courserequest_testcase extends advanced_testcase { $data->reason = 'Because PHP Unit is cool.'; // Test without category. + $this->setUser($requester); $cr = course_request::create($data); + $this->setAdminUser(); + $sink = $this->redirectMessages(); $id = $cr->approve(); - $this->assertDebuggingCalled(); // Caused by sending of message. + $this->assertCount(1, $sink->get_messages()); + $sink->close(); $course = $DB->get_record('course', array('id' => $id)); $this->assertEquals($data->fullname, $course->fullname); $this->assertEquals($data->shortname, $course->shortname); @@ -115,9 +122,13 @@ class core_course_courserequest_testcase extends advanced_testcase { set_config('defaultrequestcategory', $cat2->id); $data->shortname .= ' 2nd'; $data->category = $cat1->id; + $this->setUser($requester); $cr = course_request::create($data); + $this->setAdminUser(); + $sink = $this->redirectMessages(); $id = $cr->approve(); - $this->assertDebuggingCalled(); // Caused by sending of message. + $this->assertCount(1, $sink->get_messages()); + $sink->close(); $course = $DB->get_record('course', array('id' => $id)); $this->assertEquals($data->category, $course->category); } @@ -126,11 +137,16 @@ class core_course_courserequest_testcase extends advanced_testcase { global $DB; $this->resetAfterTest(true); $this->preventResetByRollback(); + + unset_config('noemailever'); + $this->setAdminUser(); set_config('enablecourserequests', 1); set_config('requestcategoryselection', 0); set_config('defaultrequestcategory', $DB->get_field_select('course_categories', "MIN(id)", "parent=0")); + $requester = $this->getDataGenerator()->create_user(); + $data = new stdClass(); $data->fullname = 'Həllo World!'; $data->shortname = 'Hi th€re!'; @@ -138,11 +154,15 @@ class core_course_courserequest_testcase extends advanced_testcase { $data->summary_editor['format'] = FORMAT_HTML; $data->reason = 'Because PHP Unit is cool.'; + $this->setUser($requester); $cr = course_request::create($data); $this->assertTrue($DB->record_exists('course_request', array('id' => $cr->id))); - $cr->reject('Sorry!'); - $this->assertDebuggingCalled(); // Caused by sending of message. - $this->assertFalse($DB->record_exists('course_request', array('id' => $cr->id))); - } + $this->setAdminUser(); + $sink = $this->redirectMessages(); + $cr->reject('Sorry!'); + $this->assertFalse($DB->record_exists('course_request', array('id' => $cr->id))); + $this->assertCount(1, $sink->get_messages()); + $sink->close(); + } } diff --git a/lib/badgeslib.php b/lib/badgeslib.php index 815fb2acfee..26d44478fb9 100644 --- a/lib/badgeslib.php +++ b/lib/badgeslib.php @@ -411,9 +411,7 @@ class badge { $pathhash = badges_bake($issued->uniquehash, $this->id, $userid, true); // Notify recipients and badge creators. - if (empty($CFG->noemailever)) { - badges_notify_badge_award($this, $userid, $issued->uniquehash, $pathhash); - } + badges_notify_badge_award($this, $userid, $issued->uniquehash, $pathhash); } } } @@ -634,7 +632,9 @@ function badges_notify_badge_award(badge $badge, $userid, $issued, $filepathhash $message = badge_message_from_template($badge->message, $params); $plaintext = format_text_email($message, FORMAT_HTML); - if ($badge->attachment && $filepathhash) { + // TODO: $filepathhash may be moodle_url instance too... + + if ($badge->attachment && is_string($filepathhash)) { $fs = get_file_storage(); $file = $fs->get_file_by_hash($filepathhash); $attachment = $file->copy_content_to_temp(); diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 27ee39ba6d9..cf389e09c0f 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -5605,32 +5605,24 @@ function email_to_user($user, $from, $subject, $messagetext, $messagehtml = '', global $CFG; - if (empty($user) || empty($user->email)) { - $nulluser = 'User is null or has no email'; - error_log($nulluser); - if (CLI_SCRIPT) { - mtrace('Error: lib/moodlelib.php email_to_user(): '.$nulluser); - } + if (empty($user) or empty($user->id)) { + debugging('Can not send email to null user', DEBUG_DEVELOPER); + return false; + } + + if (empty($user->email)) { + debugging('Can not send email to user without email: '.$user->id, DEBUG_DEVELOPER); return false; } if (!empty($user->deleted)) { - // Do not mail deleted users. - $userdeleted = 'User is deleted'; - error_log($userdeleted); - if (CLI_SCRIPT) { - mtrace('Error: lib/moodlelib.php email_to_user(): '.$userdeleted); - } + debugging('Can not send email to deleted user: '.$user->id, DEBUG_DEVELOPER); return false; } if (!empty($CFG->noemailever)) { // Hidden setting for development sites, set in config.php if needed. - $noemail = 'Not sending email due to noemailever config setting'; - error_log($noemail); - if (CLI_SCRIPT) { - mtrace('Error: lib/moodlelib.php email_to_user(): '.$noemail); - } + debugging('Not sending email due to $CFG->noemailever config setting', DEBUG_NORMAL); return true; } diff --git a/lib/phpmailer/moodle_phpmailer.php b/lib/phpmailer/moodle_phpmailer.php index 881b1f8126d..d58da430415 100644 --- a/lib/phpmailer/moodle_phpmailer.php +++ b/lib/phpmailer/moodle_phpmailer.php @@ -147,7 +147,7 @@ class moodle_phpmailer extends PHPMailer { // Now ask phpunit if it wants to catch this message. if (PHPUNIT_TEST) { if (!phpunit_util::is_redirecting_phpmailer()) { - debugging('Unit tests must not send real emails! Use $this->start_phpmailer_redirection()'); + debugging('Unit tests must not send real emails! Use $this->redirectEmails()'); return true; } $mail = new stdClass(); diff --git a/lib/tests/authlib_test.php b/lib/tests/authlib_test.php index 2abcd82d96e..6d1e1e55088 100644 --- a/lib/tests/authlib_test.php +++ b/lib/tests/authlib_test.php @@ -36,8 +36,7 @@ class core_authlib_testcase extends advanced_testcase { $this->resetAfterTest(); - $oldlog = ini_get('error_log'); - ini_set('error_log', "$CFG->dataroot/testlog.log"); // Prevent standard logging. + unset_config('noemailever'); set_config('lockoutthreshold', 0); set_config('lockoutwindow', 60*20); @@ -60,10 +59,10 @@ class core_authlib_testcase extends advanced_testcase { login_attempt_failed($user); login_attempt_failed($user); $this->assertFalse(login_is_lockedout($user)); - ob_start(); + $sink = $this->redirectEmails(); login_attempt_failed($user); - $output = ob_get_clean(); - $this->assertContains('noemailever', $output); + $this->assertCount(1, $sink->get_messages()); + $sink->close(); $this->assertTrue(login_is_lockedout($user)); // Test unlock works. @@ -90,10 +89,10 @@ class core_authlib_testcase extends advanced_testcase { // Test lock duration works. - ob_start(); // Prevent nomailever notice. + $sink = $this->redirectEmails(); login_attempt_failed($user); - $output = ob_get_clean(); - $this->assertContains('noemailever', $output); + $this->assertCount(1, $sink->get_messages()); + $sink->close(); $this->assertTrue(login_is_lockedout($user)); set_user_preference('login_lockout', time()-60*30+10, $user); $this->assertTrue(login_is_lockedout($user)); @@ -108,8 +107,6 @@ class core_authlib_testcase extends advanced_testcase { login_attempt_failed($user); login_attempt_failed($user); $this->assertFalse(login_is_lockedout($user)); - - ini_set('error_log', $oldlog); } public function test_authenticate_user_login() { @@ -117,8 +114,7 @@ class core_authlib_testcase extends advanced_testcase { $this->resetAfterTest(); - $oldlog = ini_get('error_log'); - ini_set('error_log', "$CFG->dataroot/testlog.log"); // Prevent standard logging. + unset_config('noemailever'); set_config('lockoutthreshold', 0); set_config('lockoutwindow', 60*20); @@ -168,9 +164,10 @@ class core_authlib_testcase extends advanced_testcase { $result = authenticate_user_login('username1', 'nopass', false, $reason); $this->assertFalse($result); $this->assertEquals(AUTH_LOGIN_FAILED, $reason); - ob_start(); // Prevent nomailever notice. + $sink = $this->redirectEmails(); $result = authenticate_user_login('username1', 'nopass', false, $reason); - ob_end_clean(); + $this->assertCount(1, $sink->get_messages()); + $sink->close(); $this->assertFalse($result); $this->assertEquals(AUTH_LOGIN_FAILED, $reason); @@ -181,8 +178,6 @@ class core_authlib_testcase extends advanced_testcase { $result = authenticate_user_login('username1', 'password1', true, $reason); $this->assertInstanceOf('stdClass', $result); $this->assertEquals(AUTH_LOGIN_OK, $reason); - - ini_set('error_log', $oldlog); } public function test_user_loggedin_event_exceptions() { diff --git a/lib/tests/messagelib_test.php b/lib/tests/messagelib_test.php index c8a60238370..4f95515dd44 100644 --- a/lib/tests/messagelib_test.php +++ b/lib/tests/messagelib_test.php @@ -148,7 +148,7 @@ class core_messagelib_testcase extends advanced_testcase { // Set config setting to allow attachments. $CFG->allowattachments = true; - $CFG->noemailever = false; + unset_config('noemailever'); $user = $this->getDataGenerator()->create_user(); $context = context_user::instance($user->id); diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index ea638bcf0f4..1a81bc9efb7 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -2493,6 +2493,8 @@ class core_moodlelib_testcase extends advanced_testcase { } public function test_email_to_user() { + global $CFG; + $this->resetAfterTest(); $user1 = $this->getDataGenerator()->create_user(); @@ -2503,8 +2505,15 @@ class core_moodlelib_testcase extends advanced_testcase { $subject2 = 'subject 2'; $messagetext2 = 'message text 2'; + $this->assertNotEmpty($CFG->noemailever); + email_to_user($user1, $user2, $subject, $messagetext); + $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting'); + unset_config('noemailever'); + email_to_user($user1, $user2, $subject, $messagetext); + $this->assertDebuggingCalled('Unit tests must not send real emails! Use $this->redirectEmails()'); + $sink = $this->redirectEmails(); email_to_user($user1, $user2, $subject, $messagetext); email_to_user($user2, $user1, $subject2, $messagetext2); @@ -2522,5 +2531,8 @@ class core_moodlelib_testcase extends advanced_testcase { $this->assertSame($messagetext2, trim($result[1]->body)); $this->assertSame($user2->email, $result[1]->to); $this->assertSame($user1->email, $result[1]->from); + + email_to_user($user1, $user2, $subject, $messagetext); + $this->assertDebuggingCalled('Unit tests must not send real emails! Use $this->redirectEmails()'); } } diff --git a/message/output/email/message_output_email.php b/message/output/email/message_output_email.php index dd59c422902..cfe9fd460cf 100644 --- a/message/output/email/message_output_email.php +++ b/message/output/email/message_output_email.php @@ -39,11 +39,8 @@ class message_output_email extends message_output { function send_message($eventdata) { global $CFG; - if (!empty($CFG->noemailever)) { - // hidden setting for development sites, set in config.php if needed - debugging('$CFG->noemailever active, no email message sent.', DEBUG_MINIMAL); - return true; - } + // Ignore $CFG->noemailever here because we want to test this code, + // the message sending fails later in email_to_user(). // skip any messaging suspended and deleted users if ($eventdata->userto->auth === 'nologin' or $eventdata->userto->suspended or $eventdata->userto->deleted) { diff --git a/message/output/jabber/message_output_jabber.php b/message/output/jabber/message_output_jabber.php index 199e2aad849..7556cd73da7 100644 --- a/message/output/jabber/message_output_jabber.php +++ b/message/output/jabber/message_output_jabber.php @@ -43,14 +43,19 @@ class message_output_jabber extends message_output { function send_message($eventdata){ global $CFG; - if (!empty($CFG->noemailever)) { - // hidden setting for development sites, set in config.php if needed - debugging('$CFG->noemailever active, no jabber message sent.', DEBUG_MINIMAL); + // Skip any messaging of suspended and deleted users. + if ($eventdata->userto->auth === 'nologin' or $eventdata->userto->suspended or $eventdata->userto->deleted) { return true; } - // skip any messaging suspended and deleted users - if ($eventdata->userto->auth === 'nologin' or $eventdata->userto->suspended or $eventdata->userto->deleted) { + if (!empty($CFG->noemailever)) { + // hidden setting for development sites, set in config.php if needed + debugging('$CFG->noemailever is active, no jabber message sent.', DEBUG_MINIMAL); + return true; + } + + if (PHPUNIT_TEST) { + // No connection to external servers allowed in phpunit tests. return true; }