From 685daf1aed66f0c6f3aea0d5fb3b60b302651695 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 1 May 2017 13:12:47 +0800 Subject: [PATCH] MDL-58650 core_message: always use 'popup' processor for messages --- lib/classes/message/manager.php | 7 ++-- lib/messagelib.php | 19 +++++++-- lib/tests/message_test.php | 4 +- lib/tests/messagelib_test.php | 41 ++++++++++--------- message/output/lib.php | 9 ++++ message/output/popup/message_output_popup.php | 11 +++++ 6 files changed, 64 insertions(+), 27 deletions(-) diff --git a/lib/classes/message/manager.php b/lib/classes/message/manager.php index 21e046a8624..d08489db601 100644 --- a/lib/classes/message/manager.php +++ b/lib/classes/message/manager.php @@ -133,13 +133,14 @@ class manager { return $savemessage->id; } - $processors = get_message_processors(true); - $failed = false; foreach ($processorlist as $procname) { // Let new messaging class add custom content based on the processor. $proceventdata = ($eventdata instanceof message) ? $eventdata->get_eventobject_for_processor($procname) : $eventdata; - if (!$processors[$procname]->object->send_message($proceventdata)) { + $stdproc = new \stdClass(); + $stdproc->name = $procname; + $processor = \core_message\api::get_processed_processor_object($stdproc); + if (!$processor->object->send_message($proceventdata)) { debugging('Error calling message processor ' . $procname); $failed = true; // Previously the $messageid = false here was overridden diff --git a/lib/messagelib.php b/lib/messagelib.php index 5347f173b61..d575cbc82aa 100644 --- a/lib/messagelib.php +++ b/lib/messagelib.php @@ -181,8 +181,19 @@ function message_send($eventdata) { } } - // Fetch enabled processors - $processors = get_message_processors(true); + // Fetch enabled processors. + // If we are dealing with a message some processors may want to handle it regardless of user and site settings. + if (empty($savemessage->notification)) { + $processors = array_filter(get_message_processors(false), function($processor) { + if ($processor->object->force_process_messages()) { + return true; + } + + return ($processor->enabled && $processor->configured); + }); + } else { + $processors = get_message_processors(true); + } // Preset variables $processorlist = array(); @@ -215,7 +226,9 @@ function message_send($eventdata) { } // Populate the list of processors we will be using - if ($permitted == 'forced' && $userisconfigured) { + if (empty($savemessage->notification) && $processor->object->force_process_messages()) { + $processorlist[] = $processor->name; + } else if ($permitted == 'forced' && $userisconfigured) { // An admin is forcing users to use this message processor. Use this processor unconditionally. $processorlist[] = $processor->name; } else if ($permitted == 'permitted' && $userisconfigured && !$eventdata->userto->emailstop) { diff --git a/lib/tests/message_test.php b/lib/tests/message_test.php index 2e98623f3de..d3395a788e7 100644 --- a/lib/tests/message_test.php +++ b/lib/tests/message_test.php @@ -165,7 +165,7 @@ class core_message_testcase extends advanced_testcase { $emails = $sink->get_messages(); $this->assertCount(1, $emails); $email = reset($emails); - $recordexists = $DB->record_exists('message_read', array('id' => $messageid)); + $recordexists = $DB->record_exists('message', array('id' => $messageid)); $this->assertSame(true, $recordexists); $this->assertSame($user1->email, $email->from); $this->assertSame($user2->email, $email->to); @@ -205,7 +205,7 @@ class core_message_testcase extends advanced_testcase { $emails = $sink->get_messages(); $this->assertCount(1, $emails); $email = reset($emails); - $recordexists = $DB->record_exists('message_read', array('id' => $messageid)); + $recordexists = $DB->record_exists('message', array('id' => $messageid)); $this->assertSame(true, $recordexists); $this->assertSame($user1->email, $email->from); $this->assertSame($user2->email, $email->to); diff --git a/lib/tests/messagelib_test.php b/lib/tests/messagelib_test.php index 1e51f3ebde8..051f61c0880 100644 --- a/lib/tests/messagelib_test.php +++ b/lib/tests/messagelib_test.php @@ -400,6 +400,7 @@ class core_messagelib_testcase extends advanced_testcase { $eventsink = $this->redirectEvents(); + // Will always use the pop-up processor. set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'none', $user2); $message = new \core\message\message(); @@ -484,6 +485,7 @@ class core_messagelib_testcase extends advanced_testcase { $this->assertInstanceOf('\core\event\message_viewed', $events[1]); $eventsink->clear(); + // Will always use the pop-up processor. set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'email', $user2); $message = new \core\message\message(); @@ -515,6 +517,7 @@ class core_messagelib_testcase extends advanced_testcase { $eventsink->clear(); $user2->emailstop = '0'; + // Will always use the pop-up processor. set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'email', $user2); $message = new \core\message\message(); @@ -534,19 +537,18 @@ class core_messagelib_testcase extends advanced_testcase { $emails = $sink->get_messages(); $this->assertCount(1, $emails); $email = reset($emails); - $savedmessage = $DB->get_record('message_read', array('id' => $messageid), '*', MUST_EXIST); + $savedmessage = $DB->get_record('message', array('id' => $messageid), '*', MUST_EXIST); $this->assertSame($user1->email, $email->from); $this->assertSame($user2->email, $email->to); $this->assertSame($message->subject, $email->subject); $this->assertNotEmpty($email->header); $this->assertNotEmpty($email->body); $sink->clear(); - $this->assertFalse($DB->record_exists('message', array())); + $this->assertFalse($DB->record_exists('message_read', array())); $DB->delete_records('message_read', array()); $events = $eventsink->get_events(); - $this->assertCount(2, $events); + $this->assertCount(1, $events); $this->assertInstanceOf('\core\event\message_sent', $events[0]); - $this->assertInstanceOf('\core\event\message_viewed', $events[1]); $eventsink->clear(); set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'email,popup', $user2); @@ -618,6 +620,7 @@ class core_messagelib_testcase extends advanced_testcase { } $transaction->allow_commit(); + // Will always use the pop-up processor. set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'none', $user2); $message = new \core\message\message(); @@ -643,13 +646,14 @@ class core_messagelib_testcase extends advanced_testcase { $this->assertFalse($DB->record_exists('message_read', array())); $DB->delete_records('message', array()); $events = $eventsink->get_events(); - $this->assertCount(1, $events); - $this->assertInstanceOf('\core\event\message_sent', $events[0]); + $this->assertCount(0, $events); $eventsink->clear(); $transaction->allow_commit(); $events = $eventsink->get_events(); - $this->assertCount(0, $events); + $this->assertCount(1, $events); + $this->assertInstanceOf('\core\event\message_sent', $events[0]); + // Will always use the pop-up processor. set_user_preference('message_provider_moodle_instantmessage_loggedoff', 'email', $user2); $message = new \core\message\message(); @@ -674,28 +678,26 @@ class core_messagelib_testcase extends advanced_testcase { $sink->clear(); $this->assertFalse($DB->record_exists('message_read', array())); $events = $eventsink->get_events(); - $this->assertCount(0, $events); + $this->assertCount(1, $events); + $this->assertInstanceOf('\core\event\message_sent', $events[0]); $transaction->allow_commit(); $events = $eventsink->get_events(); $this->assertCount(2, $events); - $this->assertInstanceOf('\core\event\message_sent', $events[0]); - $this->assertInstanceOf('\core\event\message_viewed', $events[1]); + $this->assertInstanceOf('\core\event\message_sent', $events[1]); $eventsink->clear(); $transaction = $DB->start_delegated_transaction(); message_send($message); message_send($message); - $this->assertCount(2, $DB->get_records('message')); - $this->assertCount(1, $DB->get_records('message_read')); + $this->assertCount(3, $DB->get_records('message')); + $this->assertFalse($DB->record_exists('message_read', array())); $events = $eventsink->get_events(); $this->assertCount(0, $events); $transaction->allow_commit(); $events = $eventsink->get_events(); - $this->assertCount(4, $events); + $this->assertCount(2, $events); $this->assertInstanceOf('\core\event\message_sent', $events[0]); - $this->assertInstanceOf('\core\event\message_viewed', $events[1]); - $this->assertInstanceOf('\core\event\message_sent', $events[2]); - $this->assertInstanceOf('\core\event\message_viewed', $events[3]); + $this->assertInstanceOf('\core\event\message_sent', $events[1]); $eventsink->clear(); $DB->delete_records('message', array()); $DB->delete_records('message_read', array()); @@ -717,10 +719,11 @@ class core_messagelib_testcase extends advanced_testcase { $this->assertCount(0, $DB->get_records('message')); $this->assertCount(0, $DB->get_records('message_read')); message_send($message); - $this->assertCount(0, $DB->get_records('message')); - $this->assertCount(1, $DB->get_records('message_read')); + $this->assertCount(1, $DB->get_records('message')); + $this->assertCount(0, $DB->get_records('message_read')); $events = $eventsink->get_events(); - $this->assertCount(2, $events); + $this->assertCount(1, $events); + $this->assertInstanceOf('\core\event\message_sent', $events[0]); $sink->clear(); $DB->delete_records('message_read', array()); } diff --git a/message/output/lib.php b/message/output/lib.php index cb81b1f74ca..ce7fd76f6ea 100644 --- a/message/output/lib.php +++ b/message/output/lib.php @@ -112,6 +112,15 @@ abstract class message_output { public function has_message_preferences() { return true; } + + /** + * Determines if this processor should process a message regardless of user preferences or site settings. + * + * @return bool + */ + public function force_process_messages() { + return false; + } } diff --git a/message/output/popup/message_output_popup.php b/message/output/popup/message_output_popup.php index 0091cec90c8..67409cc43b9 100644 --- a/message/output/popup/message_output_popup.php +++ b/message/output/popup/message_output_popup.php @@ -128,4 +128,15 @@ class message_output_popup extends message_output { $DB->update_record('message_popup', $record); } } + + /** + * Determines if this processor should process a message regardless of user preferences or site settings. + * + * @return bool + */ + public function force_process_messages() { + global $CFG; + + return !empty($CFG->messaging); + } } -- 2.17.1