MDL-58650 core_message: always use 'popup' processor for messages
authorMark Nelson <markn@moodle.com>
Mon, 1 May 2017 05:12:47 +0000 (13:12 +0800)
committerMark Nelson <markn@moodle.com>
Tue, 2 May 2017 07:46:48 +0000 (15:46 +0800)
lib/classes/message/manager.php
lib/messagelib.php
lib/tests/message_test.php
lib/tests/messagelib_test.php
message/output/lib.php
message/output/popup/message_output_popup.php

index 21e046a..d08489d 100644 (file)
@@ -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
index 5347f17..d575cbc 100644 (file)
@@ -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) {
index 2e98623..d3395a7 100644 (file)
@@ -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);
index 1e51f3e..051f61c 100644 (file)
@@ -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());
     }
index cb81b1f..ce7fd76 100644 (file)
@@ -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;
+    }
 }
 
 
index 0091cec..67409cc 100644 (file)
@@ -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);
+    }
 }