MDL-36941 core: convert existing api to use new table structure

Also deprecated the following functions -

1. message_move_userfrom_unread2read - It is not necessary
   for us to mark a message as read on user deletion.
2. message_get_blocked_users - Horrible logic used to
   determine if a user is blocked via reference on some
   randomly chosen 'isblocked' variable.
3. message_get_contacts - The same as above. This can be
   done in a much nicer way.
4. message_mark_message_read - We want two functions to do
   this to avoid confusing messages and notifications.
5. message_can_delete_message - This assumed the variable
   $message contained the 'useridto' property, which
   was present in the old table structure. We do not want
   future usages where a query is done on the new table
   and is simply passed as this won't contain this property.
6. message_delete_message - Same as above.

lib/classes/message/manager.php

@@ -54,7 +54,7 @@ class manager {
      * @param \core\message\message $eventdata fully prepared event data for processors
      * @param \stdClass $savemessage the message saved in 'message' table
      * @param array $processorlist list of processors for target user
-     * @return int $messageid the id from 'message' or 'message_read' table (false is not returned)
+     * @return int $messageid the id from 'messages' (false is not returned)
      */
     public static function send_message($eventdata, \stdClass $savemessage, array $processorlist) {
         global $CFG;
@@ -78,26 +78,26 @@ class manager {
 
         if (empty($processorlist)) {
             // Trigger event for sending a message - we need to do this before marking as read!
-            \core\event\message_sent::create_from_ids(
+            if (!$eventdata->notification) {
-                $eventdata->userfrom->id,
+                \core\event\message_sent::create_from_ids(
-                $eventdata->userto->id,
+                    $eventdata->userfrom->id,
-                $savemessage->id,
+                    $eventdata->userto->id,
-                $eventdata->courseid
+                    $savemessage->id,
+                    $eventdata->courseid
                 )->trigger();
-
-            if ($savemessage->notification or empty($CFG->messaging)) {
-                // If they have deselected all processors and its a notification mark it read. The user doesn't want to be bothered.
-                // The same goes if the messaging is completely disabled.
-                // We cannot insert directly to the message_read table because we want to get all events in proper order!
-                $messageid = message_mark_message_read($savemessage, time(), true);
-
-            } else {
-                // Just add it to the list of unread messages, there is no way it could be delivered to them,
-                // but they can read it via the messaging UI later.
-                $messageid = $savemessage->id;
             }
 
-            return $messageid;
+            if ($eventdata->notification or empty($CFG->messaging)) {
+                // If they have deselected all processors and its a notification mark it read. The user doesn't want to be bothered.
+                // The same goes if the messaging is completely disabled.
+                if ($eventdata->notification) {
+                    \core_message\api::mark_notification_as_read($eventdata->userto->id, $savemessage->id);
+                } else {
+                    \core_message\api::mark_message_as_read($eventdata->userto->id, $savemessage->id);
+                }
+            }
+
+            return $savemessage->id;
         }
 
         // Let the manager do the sending or buffering when db transaction in progress.
@@ -133,7 +133,6 @@ class manager {
             return $savemessage->id;
         }
 
-        $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;
@@ -142,40 +141,36 @@ class manager {
             $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
-                // by other processors and message_mark_message_read() below.
             }
         }
 
         // Trigger event for sending a message - must be done before marking as read.
-        \core\event\message_sent::create_from_ids(
+        if (!$eventdata->notification) {
-            $eventdata->userfrom->id,
+            \core\event\message_sent::create_from_ids(
-            $eventdata->userto->id,
+                $eventdata->userfrom->id,
-            $savemessage->id,
+                $eventdata->userto->id,
-            $eventdata->courseid
+                $savemessage->id,
+                $eventdata->courseid
             )->trigger();
-
-        if (empty($CFG->messaging)) {
-            // If messaging is disabled and they previously had forum notifications handled by the popup processor
-            // or any processor that puts a row in message_working then the notification will remain forever
-            // unread. To prevent this mark the message read if messaging is disabled.
-            $messageid = message_mark_message_read($savemessage, time());
-
-        } else if ($failed) {
-            // Something failed, better keep it as unread then.
-            $messageid = $savemessage->id;
-
-        } else if ($DB->count_records('message_working', array('unreadmessageid' => $savemessage->id)) == 0) {
-            // If there is no more processors that want to process this we can move message to message_read.
-            $messageid = message_mark_message_read($savemessage, time(), true);
-
-        } else {
-            // Some processor is still working on the data, let's keep it unread.
-            $messageid = $savemessage->id;
         }
 
-        return $messageid;
+        // If messaging is disabled and they previously had forum notifications handled by the popup processor
+        // or any processor that puts a row in message_working then the notification will remain forever
+        // unread. To prevent this mark the message read if messaging is disabled.
+        if (empty($CFG->messaging) && $eventdata->notification) {
+            \core_message\api::mark_notification_as_read($eventdata->userto->id, $savemessage->id);
+        }
+
+        // If there is no more processors that want to process this we can mark the message as read.
+        if ($DB->count_records('message_working', array('unreadmessageid' => $savemessage->id)) == 0) {
+            if ($eventdata->notification) {
+                \core_message\api::mark_notification_as_read($eventdata->userto->id, $savemessage->id);
+            } else {
+                \core_message\api::mark_message_as_read($eventdata->userto->id, $savemessage->id);
+            }
+        }
+
+        return $savemessage->id;
     }
 
     /**

lib/classes/task/messaging_cleanup_task.php

@@ -50,7 +50,7 @@ class messaging_cleanup_task extends scheduled_task {
         if (!empty($CFG->messagingdeletereadnotificationsdelay)) {
             $notificationdeletetime = $timenow - $CFG->messagingdeletereadnotificationsdelay;
             $params = array('notificationdeletetime' => $notificationdeletetime);
-            $DB->delete_records_select('message_read', 'notification=1 AND timeread<:notificationdeletetime', $params);
+            $DB->delete_records_select('notifications', 'timeread < :notificationdeletetime', $params);
         }
 
     }

lib/deprecatedlib.php

@@ -6577,3 +6577,192 @@ function question_is_only_toplevel_category_in_context($categoryid) {
 
     return question_is_only_child_of_top_category_in_context($categoryid);
 }
+
+/**
+ * Moves messages from a particular user from the message table (unread messages) to message_read
+ * This is typically only used when a user is deleted
+ *
+ * @param object $userid User id
+ * @return boolean success
+ * @deprecated since Moodle 3.5
+ */
+function message_move_userfrom_unread2read($userid) {
+    debugging('message_move_userfrom_unread2read() is deprecated and is no longer used.', DEBUG_DEVELOPER);
+
+    global $DB;
+
+    // Move all unread messages from message table to message_read.
+    if ($messages = $DB->get_records_select('message', 'useridfrom = ?', array($userid), 'timecreated')) {
+        foreach ($messages as $message) {
+            message_mark_message_read($message, 0); // Set timeread to 0 as the message was never read.
+        }
+    }
+    return true;
+}
+
+/**
+ * Retrieve users blocked by $user1
+ *
+ * @param object $user1 the user whose messages are being viewed
+ * @param object $user2 the user $user1 is talking to. If they are being blocked
+ *                      they will have a variable called 'isblocked' added to their user object
+ * @return array the users blocked by $user1
+ * @deprecated since Moodle 3.5
+ */
+function message_get_blocked_users($user1=null, $user2=null) {
+    debugging('message_get_blocked_users() is deprecated, please use \core_message\api::get_blocked_users() instead.',
+        DEBUG_DEVELOPER);
+
+    global $USER;
+
+    if (empty($user1)) {
+        $user1 = new stdClass();
+        $user1->id = $USER->id;
+    }
+
+    return \core_message\api::get_blocked_users($user1->id);
+}
+
+/**
+ * Retrieve $user1's contacts (online, offline and strangers)
+ *
+ * @param object $user1 the user whose messages are being viewed
+ * @param object $user2 the user $user1 is talking to. If they are a contact
+ *                      they will have a variable called 'iscontact' added to their user object
+ * @return array containing 3 arrays. array($onlinecontacts, $offlinecontacts, $strangers)
+ * @deprecated since Moodle 3.5
+ */
+function message_get_contacts($user1=null, $user2=null) {
+    debugging('message_get_contacts() is deprecated and is no longer used.', DEBUG_DEVELOPER);
+
+    global $DB, $CFG, $USER;
+
+    if (empty($user1)) {
+        $user1 = $USER;
+    }
+
+    if (!empty($user2)) {
+        $user2->iscontact = false;
+    }
+
+    $timetoshowusers = 300; // Seconds default.
+    if (isset($CFG->block_online_users_timetosee)) {
+        $timetoshowusers = $CFG->block_online_users_timetosee * 60;
+    }
+
+    // Rime which a user is counting as being active since.
+    $timefrom = time() - $timetoshowusers;
+
+    // People in our contactlist who are online.
+    $onlinecontacts  = array();
+    // People in our contactlist who are offline.
+    $offlinecontacts = array();
+    // People who are not in our contactlist but have sent us a message.
+    $strangers       = array();
+
+    // Get all in our contact list who are not blocked in our and count messages we have waiting from each of them.
+    $rs = \core_message\api::get_contacts_with_unread_message_count($user1->id);
+    foreach ($rs as $rd) {
+        if ($rd->lastaccess >= $timefrom) {
+            // They have been active recently, so are counted online.
+            $onlinecontacts[] = $rd;
+
+        } else {
+            $offlinecontacts[] = $rd;
+        }
+
+        if (!empty($user2) && $user2->id == $rd->id) {
+            $user2->iscontact = true;
+        }
+    }
+
+    // Get messages from anyone who isn't in our contact list and count the number of messages we have from each of them.
+    $rs = \core_message\api::get_non_contacts_with_unread_message_count($user1->id);
+    // Add user id as array index, so supportuser and noreply user don't get duplicated (if they are real users).
+    foreach ($rs as $rd) {
+        $strangers[$rd->id] = $rd;
+    }
+
+    // Add noreply user and support user to the list, if they don't exist.
+    $supportuser = core_user::get_support_user();
+    if (!isset($strangers[$supportuser->id]) && !$supportuser->deleted) {
+        $supportuser->messagecount = message_count_unread_messages($USER, $supportuser);
+        if ($supportuser->messagecount > 0) {
+            $strangers[$supportuser->id] = $supportuser;
+        }
+    }
+
+    $noreplyuser = core_user::get_noreply_user();
+    if (!isset($strangers[$noreplyuser->id]) && !$noreplyuser->deleted) {
+        $noreplyuser->messagecount = message_count_unread_messages($USER, $noreplyuser);
+        if ($noreplyuser->messagecount > 0) {
+            $strangers[$noreplyuser->id] = $noreplyuser;
+        }
+    }
+
+    return array($onlinecontacts, $offlinecontacts, $strangers);
+}
+
+/**
+ * Mark a single message as read
+ *
+ * @param stdClass $message An object with an object property ie $message->id which is an id in the message table
+ * @param int $timeread the timestamp for when the message should be marked read. Usually time().
+ * @param bool $messageworkingempty Is the message_working table already confirmed empty for this message?
+ * @return int the ID of the message in the messags table
+ * @deprecated since Moodle 3.5
+ */
+function message_mark_message_read($message, $timeread, $messageworkingempty=false) {
+    debugging('message_mark_message_read() is deprecated, please use \core_message\api::mark_message_as_read()
+        or \core_message\api::mark_notification_as_read().', DEBUG_DEVELOPER);
+
+    global $DB;
+
+    if (!empty($message->notification)) {
+        \core_message\api::mark_notification_as_read($message->useridto, $message->id, $timeread);
+    } else {
+        \core_message\api::mark_message_as_read($message->useridto, $message->id, $timeread);
+    }
+
+    // If any processors have pending actions abort them.
+    if (!$messageworkingempty) {
+        $DB->delete_records('message_working', array('unreadmessageid' => $message->id));
+    }
+
+    return $message->id;
+}
+
+
+/**
+ * Checks if a user can delete a message.
+ *
+ * @param stdClass $message the message to delete
+ * @param string $userid the user id of who we want to delete the message for (this may be done by the admin
+ *  but will still seem as if it was by the user)
+ * @return bool Returns true if a user can delete the message, false otherwise.
+ * @deprecated since Moodle 3.5
+ */
+function message_can_delete_message($message, $userid) {
+    debugging('message_can_delete_message() is deprecated, please use \core_message\api::can_delete_message() instead.',
+        DEBUG_DEVELOPER);
+
+    return \core_message\api::can_delete_message($userid, $message->id);
+}
+
+/**
+ * Deletes a message.
+ *
+ * This function does not verify any permissions.
+ *
+ * @param stdClass $message the message to delete
+ * @param string $userid the user id of who we want to delete the message for (this may be done by the admin
+ *  but will still seem as if it was by the user)
+ * @return bool
+ * @deprecated since Moodle 3.5
+ */
+function message_delete_message($message, $userid) {
+    debugging('message_delete_message() is deprecated, please use \core_message\api::delete_message() instead.',
+        DEBUG_DEVELOPER);
+
+    return \core_message\api::delete_message($userid, $message->id);
+}

lib/messagelib.php

@@ -127,33 +127,53 @@ function message_send($eventdata) {
         $userstate = 'loggedoff';
     }
 
-    // Create the message object
+    // Check if we are creating a notification or message.
-    $savemessage = new stdClass();
+    if ($eventdata->notification) {
-    $savemessage->courseid          = $eventdata->courseid;
+        $table = 'notifications';
-    $savemessage->useridfrom        = $eventdata->userfrom->id;
-    $savemessage->useridto          = $eventdata->userto->id;
-    $savemessage->subject           = $eventdata->subject;
-    $savemessage->fullmessage       = $eventdata->fullmessage;
-    $savemessage->fullmessageformat = $eventdata->fullmessageformat;
-    $savemessage->fullmessagehtml   = $eventdata->fullmessagehtml;
-    $savemessage->smallmessage      = $eventdata->smallmessage;
-    $savemessage->notification      = $eventdata->notification;
-    $savemessage->eventtype         = $eventdata->name;
-    $savemessage->component         = $eventdata->component;
 
-    if (!empty($eventdata->contexturl)) {
+        $tabledata = new stdClass();
-        $savemessage->contexturl = (string)$eventdata->contexturl;
+        $tabledata->useridfrom = $eventdata->userfrom->id;
+        $tabledata->useridto = $eventdata->userto->id;
+        $tabledata->subject = $eventdata->subject;
+        $tabledata->fullmessage = $eventdata->fullmessage;
+        $tabledata->fullmessageformat = $eventdata->fullmessageformat;
+        $tabledata->fullmessagehtml = $eventdata->fullmessagehtml;
+        $tabledata->smallmessage = $eventdata->smallmessage;
+        $tabledata->eventtype = $eventdata->name;
+        $tabledata->component = $eventdata->component;
+
+        if (!empty($eventdata->contexturl)) {
+            $tabledata->contexturl = (string)$eventdata->contexturl;
+        } else {
+            $tabledata->contexturl = null;
+        }
+
+        if (!empty($eventdata->contexturlname)) {
+            $tabledata->contexturlname = (string)$eventdata->contexturlname;
+        } else {
+            $tabledata->contexturlname = null;
+        }
     } else {
-        $savemessage->contexturl = null;
+        $table = 'messages';
+
+        if (!$conversationid = \core_message\api::get_conversation_between_users($eventdata->userfrom->id,
+                $eventdata->userto->id)) {
+            $conversationid = \core_message\api::create_conversation_between_users($eventdata->userfrom->id,
+                $eventdata->userto->id);
+        }
+
+        $tabledata = new stdClass();
+        $tabledata->courseid = $eventdata->courseid;
+        $tabledata->useridfrom = $eventdata->userfrom->id;
+        $tabledata->conversationid = $conversationid;
+        $tabledata->subject = $eventdata->subject;
+        $tabledata->fullmessage = $eventdata->fullmessage;
+        $tabledata->fullmessageformat = $eventdata->fullmessageformat;
+        $tabledata->fullmessagehtml = $eventdata->fullmessagehtml;
+        $tabledata->smallmessage = $eventdata->smallmessage;
     }
 
-    if (!empty($eventdata->contexturlname)) {
+    $tabledata->timecreated = time();
-        $savemessage->contexturlname = (string)$eventdata->contexturlname;
-    } else {
-        $savemessage->contexturlname = null;
-    }
-
-    $savemessage->timecreated = time();
 
     if (PHPUNIT_TEST and class_exists('phpunit_util')) {
         // Add some more tests to make sure the normal code can actually work.
@@ -173,9 +193,21 @@ function message_send($eventdata) {
         unset($messageproviders);
         // Now ask phpunit if it wants to catch this message.
         if (phpunit_util::is_redirecting_messages()) {
-            $savemessage->timeread = time();
+            $messageid = $DB->insert_record($table, $tabledata);
-            $messageid = $DB->insert_record('message_read', $savemessage);
+            $message = $DB->get_record($table, array('id' => $messageid));
-            $message = $DB->get_record('message_read', array('id'=>$messageid));
+
+            // Add the useridto attribute for BC.
+            $message->useridto = $eventdata->userto->id;
+
+            // Mark the message/notification as read.
+            if ($eventdata->notification) {
+                \core_message\api::mark_notification_as_read($eventdata->userto->id, $message->id);
+            } else {
+                \core_message\api::mark_message_as_read($eventdata->userto->id, $message->id);
+            }
+
+            // Unit tests need this detail.
+            $message->notification = $eventdata->notification;
             phpunit_util::message_sent($message);
             return $messageid;
         }
@@ -183,7 +215,7 @@ function message_send($eventdata) {
 
     // 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)) {
+    if (!$eventdata->notification) {
         $processors = array_filter(get_message_processors(false), function($processor) {
             if ($processor->object->force_process_messages()) {
                 return true;
@@ -226,7 +258,7 @@ function message_send($eventdata) {
         }
 
         // Populate the list of processors we will be using
-        if (empty($savemessage->notification) && $processor->object->force_process_messages()) {
+        if (!$eventdata->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.
@@ -248,20 +280,20 @@ function message_send($eventdata) {
     }
 
     // Only cache messages, not notifications.
-    if (empty($savemessage->notification)) {
+    if (!$eventdata->notification) {
         // Cache the timecreated value of the last message between these two users.
         $cache = cache::make('core', 'message_time_last_message_between_users');
-        $key = \core_message\helper::get_last_message_time_created_cache_key($savemessage->useridfrom,
+        $key = \core_message\helper::get_last_message_time_created_cache_key($eventdata->userfrom->id,
-            $savemessage->useridto);
+            $eventdata->userto->id);
-        $cache->set($key, $savemessage->timecreated);
+        $cache->set($key, $tabledata->timecreated);
     }
 
     // Store unread message just in case we get a fatal error any time later.
-    $savemessage->id = $DB->insert_record('message', $savemessage);
+    $tabledata->id = $DB->insert_record($table, $tabledata);
-    $eventdata->savedmessageid = $savemessage->id;
+    $eventdata->savedmessageid = $tabledata->id;
 
     // Let the manager do the sending or buffering when db transaction in progress.
-    return \core\message\manager::send_message($eventdata, $savemessage, $processorlist);
+    return \core\message\manager::send_message($eventdata, $tabledata, $processorlist);
 }
 
 

lib/moodlelib.php

@@ -4071,9 +4071,6 @@ function delete_user(stdClass $user) {
     // Delete all grades - backup is kept in grade_grades_history table.
     grade_user_delete($user->id);
 
-    // Move unread messages from this user to read.
-    message_move_userfrom_unread2read($user->id);
-
     // TODO: remove from cohorts using standard API here.
 
     // Remove user tags.

lib/phpunit/classes/message_sink.php

@@ -33,7 +33,7 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class phpunit_message_sink {
-    /** @var array of records from message_read table */
+    /** @var array of records from messages table */
     protected $messages = array();
 
     /**
@@ -48,7 +48,7 @@ class phpunit_message_sink {
     /**
      * To be called from phpunit_util only!
      *
-     * @param stdClass $message record from message_read table
+     * @param stdClass $message record from messages table
      */
     public function add_message($message) {
         /* Number messages from 0. */
@@ -58,7 +58,7 @@ class phpunit_message_sink {
     /**
      * Returns all redirected messages.
      *
-     * The instances are records form the message_read table.
+     * The instances are records from the messages table.
      * The array indexes are numbered from 0 and the order is matching
      * the creation of events.
      *

lib/phpunit/classes/phpmailer_sink.php

@@ -50,7 +50,7 @@ class phpunit_phpmailer_sink {
     /**
      * To be called from phpunit_util only!
      *
-     * @param stdClass $message record from message_read table
+     * @param stdClass $message record from messages table
      */
     public function add_message($message) {
         /* Number messages from 0. */
@@ -60,7 +60,7 @@ class phpunit_phpmailer_sink {
     /**
      * Returns all redirected messages.
      *
-     * The instances are records form the message_read table.
+     * The instances are records from the messages table.
      * The array indexes are numbered from 0 and the order is matching
      * the creation of events.
      *

lib/phpunit/classes/util.php

@@ -714,7 +714,7 @@ class phpunit_util extends testing_util {
     /**
      * To be called from messagelib.php only!
      *
-     * @param stdClass $message record from message_read table
+     * @param stdClass $message record from messages table
      * @return bool true means send message, false means message "sent" to sink.
      */
     public static function message_sent($message) {
@@ -765,7 +765,7 @@ class phpunit_util extends testing_util {
     /**
      * To be called from messagelib.php only!
      *
-     * @param stdClass $message record from message_read table
+     * @param stdClass $message record from messages table
      * @return bool true means send message, false means message "sent" to sink.
      */
     public static function phpmailer_sent($message) {

lib/tests/message_test.php

@@ -167,7 +167,7 @@ class core_message_testcase extends advanced_testcase {
         $emails = $sink->get_messages();
         $this->assertCount(1, $emails);
         $email = reset($emails);
-        $recordexists = $DB->record_exists('message', array('id' => $messageid));
+        $recordexists = $DB->record_exists('messages', array('id' => $messageid));
         $this->assertSame(true, $recordexists);
         $this->assertSame($user1->email, $email->from);
         $this->assertSame($user2->email, $email->to);
@@ -207,7 +207,7 @@ class core_message_testcase extends advanced_testcase {
         $emails = $sink->get_messages();
         $this->assertCount(1, $emails);
         $email = reset($emails);
-        $recordexists = $DB->record_exists('message', array('id' => $messageid));
+        $recordexists = $DB->record_exists('messages', array('id' => $messageid));
         $this->assertSame(true, $recordexists);
         $this->assertSame($user1->email, $email->from);
         $this->assertSame($user2->email, $email->to);

lib/tests/messagelib_test.php

@@ -218,14 +218,15 @@ class core_messagelib_testcase extends advanced_testcase {
         $this->assertEquals($message->smallmessage, $savedmessage->smallmessage);
         $this->assertEquals($message->smallmessage, $savedmessage->smallmessage);
         $this->assertEquals($message->notification, $savedmessage->notification);
-        $this->assertNull($savedmessage->contexturl);
-        $this->assertNull($savedmessage->contexturlname);
         $this->assertTimeCurrent($savedmessage->timecreated);
-        $record = $DB->get_record('message_read', array('id' => $savedmessage->id), '*', MUST_EXIST);
+        $record = $DB->get_record('messages', array('id' => $savedmessage->id), '*', MUST_EXIST);
+        unset($savedmessage->useridto);
+        unset($savedmessage->notification);
         $this->assertEquals($record, $savedmessage);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message', array()));
+        $this->assertTrue($DB->record_exists('message_user_actions', array('userid' => $user2->id, 'messageid' => $messageid,
-        $DB->delete_records('message_read', array());
+            'action' => \core_message\api::MESSAGE_ACTION_READ)));
+        $DB->delete_records('messages', array());
 
         $message = new \core\message\message();
         $message->courseid = 1;
@@ -239,8 +240,7 @@ class core_messagelib_testcase extends advanced_testcase {
         $message->fullmessagehtml = '<p>message body</p>';
         $message->smallmessage = 'small message';
         $message->notification = '0';
-        $message->contexturl = new moodle_url('/');
+
-        $message->contexturlname = 'front';
         $sink = $this->redirectMessages();
         $messageid = message_send($message);
         $savedmessages = $sink->get_messages();
@@ -255,14 +255,15 @@ class core_messagelib_testcase extends advanced_testcase {
         $this->assertEquals($message->smallmessage, $savedmessage->smallmessage);
         $this->assertEquals($message->smallmessage, $savedmessage->smallmessage);
         $this->assertEquals($message->notification, $savedmessage->notification);
-        $this->assertEquals($message->contexturl->out(), $savedmessage->contexturl);
-        $this->assertEquals($message->contexturlname, $savedmessage->contexturlname);
         $this->assertTimeCurrent($savedmessage->timecreated);
-        $record = $DB->get_record('message_read', array('id' => $savedmessage->id), '*', MUST_EXIST);
+        $record = $DB->get_record('messages', array('id' => $savedmessage->id), '*', MUST_EXIST);
+        unset($savedmessage->useridto);
+        unset($savedmessage->notification);
         $this->assertEquals($record, $savedmessage);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message', array()));
+        $this->assertTrue($DB->record_exists('message_user_actions', array('userid' => $user2->id, 'messageid' => $messageid,
-        $DB->delete_records('message_read', array());
+            'action' => \core_message\api::MESSAGE_ACTION_READ)));
+        $DB->delete_records('messages', array());
 
         // Test phpunit problem detection.
 
@@ -297,8 +298,7 @@ class core_messagelib_testcase extends advanced_testcase {
         }
         $this->assertCount(0, $sink->get_messages());
         $sink->close();
-        $this->assertFalse($DB->record_exists('message', array()));
+        $this->assertFalse($DB->record_exists('messages', array()));
-        $this->assertFalse($DB->record_exists('message_read', array()));
 
         // Invalid users.
 
@@ -420,10 +420,11 @@ class core_messagelib_testcase extends advanced_testcase {
         $messageid = message_send($message);
         $emails = $sink->get_messages();
         $this->assertCount(0, $emails);
-        $savedmessage = $DB->get_record('message', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message_read', array()));
+        $this->assertFalse($DB->record_exists('message_user_actions', array()));
-        $DB->delete_records('message', array());
+        $DB->delete_records('messages', array());
+        $DB->delete_records('message_user_actions', array());
         $events = $eventsink->get_events();
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
@@ -447,10 +448,12 @@ class core_messagelib_testcase extends advanced_testcase {
         $messageid = message_send($message);
         $emails = $sink->get_messages();
         $this->assertCount(0, $emails);
-        $savedmessage = $DB->get_record('message_read', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message', array()));
+        $this->assertTrue($DB->record_exists('message_user_actions', array('userid' => $user2->id, 'messageid' => $messageid,
-        $DB->delete_records('message_read', array());
+            'action' => \core_message\api::MESSAGE_ACTION_READ)));
+        $DB->delete_records('messages', array());
+        $DB->delete_records('message_user_actions', array());
         $events = $eventsink->get_events();
         $this->assertCount(2, $events);
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
@@ -475,14 +478,12 @@ class core_messagelib_testcase extends advanced_testcase {
         $messageid = message_send($message);
         $emails = $sink->get_messages();
         $this->assertCount(0, $emails);
-        $savedmessage = $DB->get_record('message_read', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('notifications', array('id' => $messageid), '*', MUST_EXIST);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message', array()));
+        $this->assertFalse($DB->record_exists('messages', array()));
-        $DB->delete_records('message_read', array());
+        $DB->delete_records('notifications', array());
         $events = $eventsink->get_events();
-        $this->assertCount(2, $events);
+        $this->assertCount(0, $events);
-        $this->assertInstanceOf('\core\event\message_sent', $events[0]);
-        $this->assertInstanceOf('\core\event\message_viewed', $events[1]);
         $eventsink->clear();
 
         // Will always use the pop-up processor.
@@ -507,10 +508,11 @@ class core_messagelib_testcase extends advanced_testcase {
         $messageid = message_send($message);
         $emails = $sink->get_messages();
         $this->assertCount(0, $emails);
-        $savedmessage = $DB->get_record('message', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message_read', array()));
+        $this->assertFalse($DB->record_exists('message_user_actions', array()));
-        $DB->delete_records('message', array());
+        $DB->delete_records('messages', array());
+        $DB->delete_records('message_user_actions', array());
         $events = $eventsink->get_events();
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
@@ -537,15 +539,15 @@ class core_messagelib_testcase extends advanced_testcase {
         $emails = $sink->get_messages();
         $this->assertCount(1, $emails);
         $email = reset($emails);
-        $savedmessage = $DB->get_record('message', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('messages', 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_read', array()));
+        $this->assertFalse($DB->record_exists('message_user_actions', array()));
-        $DB->delete_records('message_read', array());
+        $DB->delete_records('message_user_actions', array());
         $events = $eventsink->get_events();
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
@@ -570,7 +572,7 @@ class core_messagelib_testcase extends advanced_testcase {
         $emails = $sink->get_messages();
         $this->assertCount(1, $emails);
         $email = reset($emails);
-        $savedmessage = $DB->get_record('message', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $working = $DB->get_record('message_working', array('unreadmessageid' => $messageid), '*', MUST_EXIST);
         $this->assertSame($user1->email, $email->from);
         $this->assertSame($user2->email, $email->to);
@@ -578,8 +580,9 @@ class core_messagelib_testcase extends advanced_testcase {
         $this->assertNotEmpty($email->header);
         $this->assertNotEmpty($email->body);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message_read', array()));
+        $this->assertFalse($DB->record_exists('message_user_actions', array()));
-        $DB->delete_records('message', array());
+        $DB->delete_records('messages', array());
+        $DB->delete_records('message_user_actions', array());
         $events = $eventsink->get_events();
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
@@ -603,11 +606,11 @@ class core_messagelib_testcase extends advanced_testcase {
         $messageid = message_send($message);
         $emails = $sink->get_messages();
         $this->assertCount(0, $emails);
-        $savedmessage = $DB->get_record('message', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $working = $DB->get_record('message_working', array('unreadmessageid' => $messageid), '*', MUST_EXIST);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message_read', array()));
+        $this->assertFalse($DB->record_exists('message_user_actions', array()));
-        $DB->delete_records('message', array());
+        $DB->delete_records('messages', array());
         $events = $eventsink->get_events();
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
@@ -641,10 +644,10 @@ class core_messagelib_testcase extends advanced_testcase {
         $messageid = message_send($message);
         $emails = $sink->get_messages();
         $this->assertCount(0, $emails);
-        $savedmessage = $DB->get_record('message', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message_read', array()));
+        $this->assertFalse($DB->record_exists('message_user_actions', array()));
-        $DB->delete_records('message', array());
+        $DB->delete_records('messages', array());
         $events = $eventsink->get_events();
         $this->assertCount(0, $events);
         $eventsink->clear();
@@ -674,9 +677,9 @@ class core_messagelib_testcase extends advanced_testcase {
         $messageid = message_send($message);
         $emails = $sink->get_messages();
         $this->assertCount(0, $emails);
-        $savedmessage = $DB->get_record('message', array('id' => $messageid), '*', MUST_EXIST);
+        $savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
         $sink->clear();
-        $this->assertFalse($DB->record_exists('message_read', array()));
+        $this->assertFalse($DB->record_exists('message_user_actions', array()));
         $events = $eventsink->get_events();
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
@@ -689,8 +692,8 @@ class core_messagelib_testcase extends advanced_testcase {
         $transaction = $DB->start_delegated_transaction();
         message_send($message);
         message_send($message);
-        $this->assertCount(3, $DB->get_records('message'));
+        $this->assertCount(3, $DB->get_records('messages'));
-        $this->assertFalse($DB->record_exists('message_read', array()));
+        $this->assertFalse($DB->record_exists('message_user_actions', array()));
         $events = $eventsink->get_events();
         $this->assertCount(0, $events);
         $transaction->allow_commit();
@@ -699,14 +702,13 @@ class core_messagelib_testcase extends advanced_testcase {
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
         $this->assertInstanceOf('\core\event\message_sent', $events[1]);
         $eventsink->clear();
-        $DB->delete_records('message', array());
+        $DB->delete_records('messages', array());
-        $DB->delete_records('message_read', array());
 
         $transaction = $DB->start_delegated_transaction();
         message_send($message);
         message_send($message);
-        $this->assertCount(2, $DB->get_records('message'));
+        $this->assertCount(2, $DB->get_records('messages'));
-        $this->assertCount(0, $DB->get_records('message_read'));
+        $this->assertCount(0, $DB->get_records('message_user_actions'));
         $events = $eventsink->get_events();
         $this->assertCount(0, $events);
         try {
@@ -716,16 +718,14 @@ class core_messagelib_testcase extends advanced_testcase {
         }
         $events = $eventsink->get_events();
         $this->assertCount(0, $events);
-        $this->assertCount(0, $DB->get_records('message'));
+        $this->assertCount(0, $DB->get_records('messages'));
-        $this->assertCount(0, $DB->get_records('message_read'));
         message_send($message);
-        $this->assertCount(1, $DB->get_records('message'));
+        $this->assertCount(1, $DB->get_records('messages'));
-        $this->assertCount(0, $DB->get_records('message_read'));
+        $this->assertCount(0, $DB->get_records('message_user_actions'));
         $events = $eventsink->get_events();
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\core\event\message_sent', $events[0]);
         $sink->clear();
-        $DB->delete_records('message_read', array());
     }
 
     public function test_rollback() {

message/classes/api.php

@@ -36,6 +36,16 @@ require_once($CFG->dirroot . '/lib/messagelib.php');
  */
 class api {
 
+    /**
+     * The action for reading a message.
+     */
+    const MESSAGE_ACTION_READ = 1;
+
+    /**
+     * The action for deleting a message.
+     */
+    const MESSAGE_ACTION_DELETED = 2;
+
     /**
      * Handles searching for messages in the message area.
      *
@@ -52,47 +62,33 @@ class api {
         $ufields = \user_picture::fields('u', array('lastaccess'), 'userfrom_id', 'userfrom_');
         $ufields2 = \user_picture::fields('u2', array('lastaccess'), 'userto_id', 'userto_');
 
-        // Get all the messages for the user.
+        $sql = "SELECT m.id, m.useridfrom, mcm.userid as useridto, m.subject, m.fullmessage, m.fullmessagehtml, m.fullmessageformat,
-        $sql = "SELECT m.id, m.useridfrom, m.useridto, m.subject, m.fullmessage, m.fullmessagehtml, m.fullmessageformat,
+                       m.smallmessage, m.timecreated, 0 as isread, $ufields, mcont.blocked as userfrom_blocked, $ufields2,
-                       m.smallmessage, m.notification, m.timecreated, 0 as isread, $ufields, mc.blocked as userfrom_blocked,
+                       mcont2.blocked as userto_blocked
-                       $ufields2, mc2.blocked as userto_blocked
+                  FROM {messages} m
-                  FROM {message} m
+            INNER JOIN {user} u
-                  JOIN {user} u
+                    ON u.id = m.useridfrom
-                    ON m.useridfrom = u.id
+            INNER JOIN {message_conversations} mc
-             LEFT JOIN {message_contacts} mc
+                    ON mc.id = m.conversationid
-                    ON (mc.contactid = u.id AND mc.userid = ?)
+            INNER JOIN {message_conversation_members} mcm
-                  JOIN {user} u2
+                    ON mcm.conversationid = m.conversationid
-                    ON m.useridto = u2.id
+            INNER JOIN {user} u2
-             LEFT JOIN {message_contacts} mc2
+                    ON u2.id = mcm.userid
-                    ON (mc2.contactid = u2.id AND mc2.userid = ?)
+             LEFT JOIN {message_contacts} mcont
-                 WHERE ((useridto = ? AND timeusertodeleted = 0)
+                    ON (mcont.contactid = u.id AND mcont.userid = ?)
-                    OR (useridfrom = ? AND timeuserfromdeleted = 0))
+             LEFT JOIN {message_contacts} mcont2
-                   AND notification = 0
+                    ON (mcont2.contactid = u2.id AND mcont2.userid = ?)
-                   AND u.deleted = 0
+             LEFT JOIN {message_user_actions} mua
-                   AND u2.deleted = 0
+                    ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ?)
-                   AND " . $DB->sql_like('smallmessage', '?', false) . "
+                 WHERE (m.useridfrom = ? OR mcm.userid = ?)
-             UNION ALL
+                   AND m.useridfrom != mcm.userid
-                SELECT mr.id, mr.useridfrom, mr.useridto, mr.subject, mr.fullmessage, mr.fullmessagehtml, mr.fullmessageformat,
-                       mr.smallmessage, mr.notification, mr.timecreated, 1 as isread, $ufields, mc.blocked as userfrom_blocked,
-                       $ufields2, mc2.blocked as userto_blocked
-                  FROM {message_read} mr
-                  JOIN {user} u
-                    ON mr.useridfrom = u.id
-             LEFT JOIN {message_contacts} mc
-                    ON (mc.contactid = u.id AND mc.userid = ?)
-                  JOIN {user} u2
-                    ON mr.useridto = u2.id
-             LEFT JOIN {message_contacts} mc2
-                    ON (mc2.contactid = u2.id AND mc2.userid = ?)
-                 WHERE ((useridto = ? AND timeusertodeleted = 0)
-                    OR (useridfrom = ? AND timeuserfromdeleted = 0))
-                   AND notification = 0
                    AND u.deleted = 0
                    AND u2.deleted = 0
+                   AND mua.id is NULL
                    AND " . $DB->sql_like('smallmessage', '?', false) . "
               ORDER BY timecreated DESC";
-        $params = array($userid, $userid, $userid, $userid, '%' . $search . '%',
+
-                        $userid, $userid, $userid, $userid, '%' . $search . '%');
+        $params = array($userid, $userid, $userid, self::MESSAGE_ACTION_DELETED, $userid, $userid, '%' . $search . '%');
 
         // Convert the messages into searchable contacts with their last message being the message that was searched.
         $conversations = array();
@@ -266,173 +262,48 @@ class api {
     public static function get_conversations($userid, $limitfrom = 0, $limitnum = 20) {
         global $DB;
 
-        // The case statement is used to make sure the same key is generated
+        // Get the last message from each conversation that the user belongs to.
-        // whether a user sent or received a message (it's the same conversation).
+        $sql = "SELECT m.id, m.conversationid, m.useridfrom, mcm2.userid as useridto, m.smallmessage, m.timecreated
-        // E.g. If there is a message from user 1 to user 2 and then from user 2 to user 1 the result set
+                  FROM {messages} m
-        // will group those into a single record, since 1 -> 2 and 2 -> 1 is the same conversation.
+            INNER JOIN (
-        $case1 = $DB->sql_concat('useridfrom', "'-'", 'useridto');
+                          SELECT MAX(m.id) AS messageid
-        $case2 = $DB->sql_concat('useridto', "'-'", 'useridfrom');
+                            FROM {messages} m
-        $convocase = "CASE WHEN useridfrom > useridto
+                      INNER JOIN (
-                        THEN $case1
+                                      SELECT m.conversationid, MAX(m.timecreated) as maxtime
-                        ELSE $case2 END";
+                                        FROM {messages} m
-        $convosig = "$convocase AS convo_signature";
+                                  INNER JOIN {message_conversation_members} mcm
+                                          ON mcm.conversationid = m.conversationid
+                                   LEFT JOIN {message_user_actions} mua
+                                          ON (mua.messageid = m.id AND mua.userid = :userid AND mua.action = :action)
+                                       WHERE mua.id is NULL
+                                         AND mcm.userid = :userid2
+                                    GROUP BY m.conversationid
+                                 ) maxmessage
+                               ON maxmessage.maxtime = m.timecreated AND maxmessage.conversationid = m.conversationid
+                         GROUP BY m.conversationid
+                       ) lastmessage
+                    ON lastmessage.messageid = m.id
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = m.conversationid
+            INNER JOIN {message_conversation_members} mcm2
+                    ON mcm2.conversationid = m.conversationid
+                 WHERE mcm.userid = m.useridfrom
+                   AND mcm.id != mcm2.id
+              ORDER BY m.timecreated DESC";
+        $messageset = $DB->get_recordset_sql($sql, ['userid' => $userid, 'action' => self::MESSAGE_ACTION_DELETED,
+            'userid2' => $userid], $limitfrom, $limitnum);
 
-        // This is a snippet to join the message tables and filter out any messages the user has deleted
-        // and ignore notifications. The fields are specified by name so that the union works on MySQL.
-        $allmessages = "SELECT
-                            id, useridfrom, useridto, subject, fullmessage, fullmessageformat,
-                            fullmessagehtml, smallmessage, notification, contexturl,
-                            contexturlname, timecreated, timeuserfromdeleted, timeusertodeleted,
-                            component, eventtype, 0 as timeread
-                        FROM {message}
-                        WHERE
-                            (useridto = ? AND timeusertodeleted = 0 AND notification = 0)
-                        UNION ALL
-                        SELECT
-                            id, useridfrom, useridto, subject, fullmessage, fullmessageformat,
-                            fullmessagehtml, smallmessage, notification, contexturl,
-                            contexturlname, timecreated, timeuserfromdeleted, timeusertodeleted,
-                            component, eventtype, 0 as timeread
-                        FROM {message}
-                        WHERE
-                            (useridfrom = ? AND timeuserfromdeleted = 0 AND notification = 0)
-                        UNION ALL
-                        SELECT
-                            id, useridfrom, useridto, subject, fullmessage, fullmessageformat,
-                            fullmessagehtml, smallmessage, notification, contexturl,
-                            contexturlname, timecreated, timeuserfromdeleted, timeusertodeleted,
-                            component, eventtype, timeread
-                        FROM {message_read}
-                        WHERE
-                            (useridto = ? AND timeusertodeleted = 0 AND notification = 0)
-                        UNION ALL
-                        SELECT
-                            id, useridfrom, useridto, subject, fullmessage, fullmessageformat,
-                            fullmessagehtml, smallmessage, notification, contexturl,
-                            contexturlname, timecreated, timeuserfromdeleted, timeusertodeleted,
-                            component, eventtype, timeread
-                        FROM {message_read}
-                        WHERE
-                            (useridfrom = ? AND timeuserfromdeleted = 0 AND notification = 0)";
-        $allmessagesparams = [$userid, $userid, $userid, $userid];
-
-        // Create a transaction to protect against concurrency issues.
-        $transaction = $DB->start_delegated_transaction();
-
-        // First we need to get the list of conversations from the database ordered by the conversation
-        // with the most recent message first.
-        //
-        // This query will join the two message tables and then group the results by the combination
-        // of useridfrom and useridto (the 'convo_signature').
-        $conversationssql = "SELECT $convosig, max(timecreated) as timecreated
-                             FROM ($allmessages) x
-                             GROUP BY $convocase
-                             ORDER BY timecreated DESC, max(id) DESC";
-        $conversationrecords = $DB->get_records_sql($conversationssql, $allmessagesparams, $limitfrom, $limitnum);
-
-        // This user has no conversations so we can return early here.
-        if (empty($conversationrecords)) {
-            $transaction->allow_commit();
-            return [];
-        }
-
-        // Next we need to get the max id of the messages sent at the latest time for each conversation.
-        // This needs to be a separate query to above because there is no guarantee that the message with
-        // the highest id will also have the highest timecreated value (in fact that is fairly likely due
-        // to the split between the message tables).
-        //
-        // E.g. if we just added max(id) to the conversation query above and ran it on data like:
-        // id, userfrom, userto, timecreated
-        //  1,        1,      2,           2
-        //  2,        2,      1,           1
-        //
-        // Then the result of the query would be:
-        // convo_signature, timecreated, id
-        //             2-1,           2,  2
-        //
-        // That would be incorrect since the message with id 2 actually has a lower timecreated. Hence why
-        // the two queries need to be split.
-        //
-        // The same result could also be achieved with an inner join in a single query however we're specifically
-        // avoiding multiple joins in the messaging queries because of the size of the messaging tables.
-        $whereclauses = [];
-        $createdtimes = [];
-        foreach ($conversationrecords as $convoid => $record) {
-            $whereclauses[] = "($convocase = '$convoid' AND timecreated = {$record->timecreated})";
-            $createdtimes[] = $record->timecreated;
-        }
-        $messageidwhere = implode(' OR ', $whereclauses);
-        list($timecreatedsql, $timecreatedparams) = $DB->get_in_or_equal($createdtimes);
-
-        $allmessagestimecreated = "SELECT id, useridfrom, useridto, timecreated
-                        FROM {message}
-                        WHERE
-                            (useridto = ? AND timeusertodeleted = 0 AND notification = 0)
-                            AND timecreated $timecreatedsql
-                        UNION ALL
-                        SELECT id, useridfrom, useridto, timecreated
-                        FROM {message}
-                        WHERE
-                            (useridfrom = ? AND timeuserfromdeleted = 0 AND notification = 0)
-                            AND timecreated $timecreatedsql
-                        UNION ALL
-                        SELECT id, useridfrom, useridto, timecreated
-                        FROM {message_read}
-                        WHERE
-                            (useridto = ? AND timeusertodeleted = 0 AND notification = 0)
-                            AND timecreated $timecreatedsql
-                        UNION ALL
-                        SELECT id, useridfrom, useridto, timecreated
-                        FROM {message_read}
-                        WHERE
-                            (useridfrom = ? AND timeuserfromdeleted = 0 AND notification = 0)
-                            AND timecreated $timecreatedsql";
-        $messageidsql = "SELECT $convosig, max(id) as id, timecreated
-                         FROM ($allmessagestimecreated) x
-                         WHERE $messageidwhere
-                         GROUP BY $convocase, timecreated";
-        $messageidparams = array_merge([$userid], $timecreatedparams, [$userid], $timecreatedparams,
-                [$userid], $timecreatedparams, [$userid], $timecreatedparams);
-        $messageidrecords = $DB->get_records_sql($messageidsql, $messageidparams);
-
-        // Ok, let's recap. We've pulled a descending ordered list of conversations by latest time created
-        // for the given user. For each of those conversations we've grabbed the max id for messages
-        // created at that time.
-        //
-        // So at this point we have the list of ids for the most recent message in each of the user's most
-        // recent conversations. Now we need to pull all of the message and user data for each message id.
-        $whereclauses = [];
-        foreach ($messageidrecords as $record) {
-            $whereclauses[] = "(id = {$record->id} AND timecreated = {$record->timecreated})";
-        }
-        $messagewhere = implode(' OR ', $whereclauses);
-        $messagesunionsql = "SELECT
-                                id, useridfrom, useridto, smallmessage, 0 as timeread
-                            FROM {message}
-                            WHERE
-                                {$messagewhere}
-                            UNION ALL
-                            SELECT
-                                id, useridfrom, useridto, smallmessage, timeread
-                            FROM {message_read}
-                            WHERE
-                                {$messagewhere}";
-        $messagesql = "SELECT $convosig, m.smallmessage, m.id, m.useridto, m.useridfrom, m.timeread
-                       FROM ($messagesunionsql) m";
-
-        // We need to handle the case where the $messageids contains two ids from the same conversation
-        // (which can happen because there can be id clashes between the read and unread tables). In
-        // this case we will prioritise the unread message.
-        $messageset = $DB->get_recordset_sql($messagesql, $allmessagesparams);
         $messages = [];
         foreach ($messageset as $message) {
-            $id = $message->convo_signature;
+            $messages[$message->id] = $message;
-            if (!isset($messages[$id]) || empty($message->timeread)) {
-                $messages[$id] = $message;
-            }
         }
         $messageset->close();
 
+        // If there are no messages return early.
+        if (empty($messages)) {
+            return [];
+        }
+
         // We need to pull out the list of other users that are part of each of these conversations. This
         // needs to be done in a separate query to avoid doing a join on the messages tables and the user
         // tables because on large sites these tables are massive which results in extremely slow
@@ -441,77 +312,64 @@ class api {
             return ($message->useridfrom == $userid) ? $message->useridto : $message->useridfrom;
         }, array_values($messages));
 
+        // Ok, let's get the other members in the conversations.
         list($useridsql, $usersparams) = $DB->get_in_or_equal($otheruserids);
-        $userfields = \user_picture::fields('', array('lastaccess'));
+        $userfields = \user_picture::fields('u', array('lastaccess'));
         $userssql = "SELECT $userfields
-                     FROM {user}
+                       FROM {user} u
-                     WHERE id $useridsql
+                      WHERE id $useridsql
-                       AND deleted = 0";
+                        AND deleted = 0";
         $otherusers = $DB->get_records_sql($userssql, $usersparams);
 
-        // Similar to the above use case, we need to pull the contact information and again this has
+        // If there are no other users (user may have been deleted), then do not continue.
-        // specifically been separated into another query to avoid having to do joins on the message
+        if (empty($otherusers)) {
-        // tables.
+            return [];
-        $contactssql = "SELECT contactid, blocked
-                        FROM {message_contacts}
-                        WHERE userid = ? AND contactid $useridsql";
-        $contacts = $DB->get_records_sql($contactssql, array_merge([$userid], $otheruserids));
-
-        // Finally, let's get the unread messages count for this user so that we can add them
-        // to the conversation.
-        $unreadcountssql = 'SELECT useridfrom, count(*) as count
-                            FROM {message}
-                            WHERE useridto = ?
-                                AND timeusertodeleted = 0
-                                AND notification = 0
-                            GROUP BY useridfrom';
-        $unreadcounts = $DB->get_records_sql($unreadcountssql, [$userid]);
-
-        // We can close off the transaction now.
-        $transaction->allow_commit();
-
-        // Now we need to order the messages back into the same order of the conversations.
-        $orderedconvosigs = array_keys($conversationrecords);
-        usort($messages, function($a, $b) use ($orderedconvosigs) {
-            $aindex = array_search($a->convo_signature, $orderedconvosigs);
-            $bindex = array_search($b->convo_signature, $orderedconvosigs);
-
-            return ($aindex < $bindex) ? -1 : 1;
-        });
-
-        // Preload the contexts before we construct the conversation to prevent the
-        // create_contact helper from needing to query the DB so often.
-        $ctxselect = \context_helper::get_preload_record_columns_sql('ctx');
-        $sql = "SELECT {$ctxselect}
-                FROM {context} ctx
-                WHERE ctx.contextlevel = ? AND
-                ctx.instanceid {$useridsql}";
-        $contexts = [];
-        $contexts = $DB->get_records_sql($sql, array_merge([CONTEXT_USER], $usersparams));
-        foreach ($contexts as $context) {
-            \context_helper::preload_from_record($context);
         }
 
+        $contactssql = "SELECT contactid, blocked
+                          FROM {message_contacts}
+                         WHERE userid = ?
+                           AND contactid $useridsql";
+        $contacts = $DB->get_records_sql($contactssql, array_merge([$userid], $usersparams));
+
+        // Finally, let's get the unread messages count for this user so that we can add them
+        // to the conversation. Remember we need to ignore the messages the user sent.
+        $unreadcountssql = 'SELECT m.useridfrom, count(m.id) as count
+                              FROM {messages} m
+                        INNER JOIN {message_conversations} mc
+                                ON mc.id = m.conversationid
+                        INNER JOIN {message_conversation_members} mcm
+                                ON m.conversationid = mcm.conversationid
+                         LEFT JOIN {message_user_actions} mua
+                                ON (mua.messageid = m.id AND mua.userid = ? AND
+                                   (mua.action = ? OR mua.action = ?))
+                             WHERE mcm.userid = ?
+                               AND m.useridfrom != ?
+                               AND mua.id is NULL
+                          GROUP BY useridfrom';
+        $unreadcounts = $DB->get_records_sql($unreadcountssql, [$userid, self::MESSAGE_ACTION_READ, self::MESSAGE_ACTION_DELETED,
+            $userid, $userid]);
+
+        // Get rid of the table prefix.
+        $userfields = str_replace('u.', '', $userfields);
         $userproperties = explode(',', $userfields);
         $arrconversations = array();
-        // The last step now is to bring all of the data we've gathered together to create
-        // a conversation (or contact, as the API is named...).
         foreach ($messages as $message) {
             $conversation = new \stdClass();
             $otheruserid = ($message->useridfrom == $userid) ? $message->useridto : $message->useridfrom;
             $otheruser = isset($otherusers[$otheruserid]) ? $otherusers[$otheruserid] : null;
             $contact = isset($contacts[$otheruserid]) ? $contacts[$otheruserid] : null;
 
+            // It's possible the other user was deleted, so, skip.
+            if (is_null($otheruser)) {
+                continue;
+            }
+
             // Add the other user's information to the conversation, if we have one.
             foreach ($userproperties as $prop) {
                 $conversation->$prop = ($otheruser) ? $otheruser->$prop : null;
             }
 
-            // Do not process a conversation with a deleted user.
-            if (empty($conversation->id)) {
-                continue;
-            }
-
             // Add the contact's information, if we have one.
             $conversation->blocked = ($contact) ? $contact->blocked : null;
 
@@ -562,6 +420,73 @@ class api {
         return $arrcontacts;
     }
 
+    /**
+     * Returns the an array of the users the given user is in a conversation
+     * with who are a contact and the number of unread messages.
+     *
+     * @param int $userid The user id
+     * @param int $limitfrom
+     * @param int $limitnum
+     * @return array
+     */
+    public static function get_contacts_with_unread_message_count($userid, $limitfrom = 0, $limitnum = 0) {
+        global $DB;
+
+        $userfields = \user_picture::fields('u', array('lastaccess'));
+        $unreadcountssql = "SELECT $userfields, count(m.id) as messagecount
+                              FROM {message_contacts} mc
+                        INNER JOIN {user} u
+                                ON u.id = mc.contactid
+                         LEFT JOIN {messages} m
+                                ON m.useridfrom = mc.contactid
+                         LEFT JOIN {message_conversation_members} mcm
+                                ON mcm.conversationid = m.conversationid AND mcm.userid = ? AND mcm.userid != m.useridfrom
+                         LEFT JOIN {message_user_actions} mua
+                                ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ?)
+                             WHERE mua.id is NULL
+                               AND mc.userid = ?
+                               AND mc.blocked = 0
+                               AND u.deleted = 0
+                          GROUP BY $userfields";
+
+        return $DB->get_records_sql($unreadcountssql, [$userid, $userid, self::MESSAGE_ACTION_READ,
+            $userid, $userid], $limitfrom, $limitnum);
+    }
+
+    /**
+     * Returns the an array of the users the given user is in a conversation
+     * with who are not a contact and the number of unread messages.
+     *
+     * @param int $userid The user id
+     * @param int $limitfrom
+     * @param int $limitnum
+     * @return array
+     */
+    public static function get_non_contacts_with_unread_message_count($userid, $limitfrom = 0, $limitnum = 0) {
+        global $DB;
+
+        $userfields = \user_picture::fields('u', array('lastaccess'));
+        $unreadcountssql = "SELECT $userfields, count(m.id) as messagecount
+                              FROM {user} u
+                        INNER JOIN {messages} m
+                                ON m.useridfrom = u.id
+                        INNER JOIN {message_conversation_members} mcm
+                                ON mcm.conversationid = m.conversationid
+                         LEFT JOIN {message_user_actions} mua
+                                ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ?)
+                         LEFT JOIN {message_contacts} mc
+                                ON (mc.userid = ? AND mc.contactid = u.id)
+                             WHERE mcm.userid = ?
+                               AND mcm.userid != m.useridfrom
+                               AND mua.id is NULL
+                               AND mc.id is NULL
+                               AND u.deleted = 0
+                          GROUP BY $userfields";
+
+        return $DB->get_records_sql($unreadcountssql, [$userid, self::MESSAGE_ACTION_READ, $userid, $userid],
+            $limitfrom, $limitnum);
+    }
+
     /**
      * Returns the messages to display in the message area.
      *
@@ -710,54 +635,43 @@ class api {
      * @return bool
      */
     public static function delete_conversation($userid, $otheruserid) {
-        global $DB;
+        global $DB, $USER;
 
-        // We need to update the tables to mark all messages as deleted from and to the other user. This seems worse than it
+        $conversationid = self::get_conversation_between_users($userid, $otheruserid);
-        // is, that's because our DB structure splits messages into two tables (great idea, huh?) which causes code like this.
-        // This won't be a particularly heavily used function (at least I hope not), so let's hope MDL-36941 gets worked on
-        // soon for the sake of any developers' sanity when dealing with the messaging system.
-        $now = time();
-        $sql = "UPDATE {message}
-                   SET timeuserfromdeleted = :time
-                 WHERE useridfrom = :userid
-                   AND useridto = :otheruserid
-                   AND notification = 0";
-        $DB->execute($sql, array('time' => $now, 'userid' => $userid, 'otheruserid' => $otheruserid));
 
-        $sql = "UPDATE {message}
+        // If there is no conversation, there is nothing to do.
-                   SET timeusertodeleted = :time
+        if (!$conversationid) {
-                 WHERE useridto = :userid
+            return true;
-                   AND useridfrom = :otheruserid
+        }
-                   AND notification = 0";
-        $DB->execute($sql, array('time' => $now, 'userid' => $userid, 'otheruserid' => $otheruserid));
 
-        $sql = "UPDATE {message_read}
+        // Get all messages belonging to this conversation that have not already been deleted by this user.
-                   SET timeuserfromdeleted = :time
+        $sql = "SELECT m.*
-                 WHERE useridfrom = :userid
+                 FROM {messages} m
-                   AND useridto = :otheruserid
+           INNER JOIN {message_conversations} mc
-                   AND notification = 0";
+                   ON m.conversationid = mc.id
-        $DB->execute($sql, array('time' => $now, 'userid' => $userid, 'otheruserid' => $otheruserid));
+            LEFT JOIN {message_user_actions} mua
+                   ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ?)
+                WHERE mua.id is NULL
+                  AND mc.id = ?
+             ORDER BY m.timecreated ASC";
+        $messages = $DB->get_records_sql($sql, [$userid, self::MESSAGE_ACTION_DELETED, $conversationid]);
 
-        $sql = "UPDATE {message_read}
+        // Ok, mark these as deleted.
-                   SET timeusertodeleted = :time
+        foreach ($messages as $message) {
-                 WHERE useridto = :userid
+            $mua = new \stdClass();
-                   AND useridfrom = :otheruserid
+            $mua->userid = $userid;
-                   AND notification = 0";
+            $mua->messageid = $message->id;
-        $DB->execute($sql, array('time' => $now, 'userid' => $userid, 'otheruserid' => $otheruserid));
+            $mua->action = self::MESSAGE_ACTION_DELETED;
+            $mua->timecreated = time();
+            $mua->id = $DB->insert_record('message_user_actions', $mua);
 
-        // Now we need to trigger events for these.
+            if ($message->useridfrom == $userid) {
-        if ($messages = helper::get_messages($userid, $otheruserid, $now)) {
+                $useridto = $otheruserid;
-            // Loop through and trigger a deleted event.
+            } else {
-            foreach ($messages as $message) {
+                $useridto = $userid;
-                $messagetable = 'message';
-                if (!empty($message->timeread)) {
-                    $messagetable = 'message_read';
-                }
-
-                // Trigger event for deleting the message.
-                \core\event\message_deleted::create_from_ids($message->useridfrom, $message->useridto,
-                    $userid, $messagetable, $message->id)->trigger();
             }
+            \core\event\message_deleted::create_from_ids($message->useridfrom, $useridto,
+                $USER->id, $message->id, $mua->id)->trigger();
         }
 
         return true;
@@ -777,11 +691,19 @@ class api {
             $user = $USER;
         }
 
-        return $DB->count_records_select(
+        $sql = "SELECT COUNT(DISTINCT(m.conversationid))
-            'message',
+                  FROM {messages} m
-            'useridto = ? AND timeusertodeleted = 0 AND notification = 0',
+            INNER JOIN {message_conversations} mc
-            [$user->id],
+                    ON m.conversationid = mc.id
-            "COUNT(DISTINCT(useridfrom))");
+            INNER JOIN {message_conversation_members} mcm
+                    ON mc.id = mcm.conversationid
+             LEFT JOIN {message_user_actions} mua
+                    ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ?)
+                 WHERE mcm.userid = ?
+                   AND mcm.userid != m.useridfrom
+                   AND mua.id is NULL";
+
+        return $DB->count_records_sql($sql, [$user->id, self::MESSAGE_ACTION_READ, $user->id]);
     }
 
     /**
@@ -797,31 +719,51 @@ class api {
     public static function mark_all_read_for_user($touserid, $fromuserid = 0, $type = '') {
         global $DB;
 
-        $params = array();
+        $type = strtolower($type);
-
-        if (!empty($touserid)) {
-            $params['useridto'] = $touserid;
-        }
 
+        // Create the SQL we will be using.
+        $messagesql = "SELECT m.*
+                         FROM {messages} m
+                   INNER JOIN {message_conversations} mc
+                           ON mc.id = m.conversationid
+                   INNER JOIN {message_conversation_members} mcm
+                           ON mcm.conversationid = mc.id
+                        WHERE mcm.userid = ?";
+        $messageparams = [$touserid];
         if (!empty($fromuserid)) {
-            $params['useridfrom'] = $fromuserid;
+            $messagesql .= " AND m.useridfrom = ?";
+            $messageparams[] = $fromuserid;
         }
 
+        $notificationsql = "SELECT n.*
+                              FROM {notifications} n
+                             WHERE useridto = ?";
+        $notificationsparams = [$touserid];
+        if (!empty($fromuserid)) {
+            $notificationsql .= " AND useridfrom = ?";
+            $notificationsparams[] = $fromuserid;
+        }
+
+        $messages = [];
+        $notifications = [];
         if (!empty($type)) {
             if (strtolower($type) == MESSAGE_TYPE_NOTIFICATION) {
-                $params['notification'] = 1;
+                $notifications = $DB->get_recordset_sql($notificationsql, $notificationsparams);
             } else if (strtolower($type) == MESSAGE_TYPE_MESSAGE) {
-                $params['notification'] = 0;
+                $messages = $DB->get_recordset_sql($messagesql, $messageparams);
             }
+        } else { // We want both.
+            $messages = $DB->get_recordset_sql($messagesql, $messageparams);
+            $notifications = $DB->get_recordset_sql($notificationsql, $notificationsparams);
         }
 
-        $messages = $DB->get_recordset('message', $params);
-
         foreach ($messages as $message) {
-            message_mark_message_read($message, time());
+            self::mark_message_as_read($touserid, $message->id);
         }
 
-        $messages->close();
+        foreach ($notifications as $notification) {
+            self::mark_notification_as_read($touserid, $notification->id);
+        }
     }
 
     /**
@@ -1076,4 +1018,240 @@ class api {
         }
         return $processor;
     }
+
+    /**
+     * Retrieve users blocked by $user1
+     *
+     * @param int $userid The user id of the user whos blocked users we are returning
+     * @return array the users blocked
+     */
+    public static function get_blocked_users($userid) {
+        global $DB;
+
+        $userfields = \user_picture::fields('u', array('lastaccess'));
+        $blockeduserssql = "SELECT $userfields
+                              FROM {message_contacts} mc
+                        INNER JOIN {user} u
+                                ON u.id = mc.contactid
+                             WHERE u.deleted = 0
+                               AND mc.userid = ?
+                               AND mc.blocked = 1
+                          GROUP BY $userfields
+                          ORDER BY u.firstname ASC";
+        return $DB->get_records_sql($blockeduserssql, [$userid]);
+    }
+
+    /**
+     * Mark a single message as read.
+     *
+     * @param int $userid The user id who marked the message as read
+     * @param int $messageid The message id
+     * @param int|null $timeread The time the message was marked as read, if null will default to time()
+     */
+    public static function mark_message_as_read($userid, $messageid, $timeread = null) {
+        global $DB;
+
+        if (is_null($timeread)) {
+            $timeread = time();
+        }
+
+        $message = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
+
+        // Check if the user has already read this message.
+        if (!$DB->record_exists('message_user_actions', ['userid' => $userid,
+                'messageid' => $messageid, 'action' => self::MESSAGE_ACTION_READ])) {
+            $mua = new \stdClass();
+            $mua->userid = $userid;
+            $mua->messageid = $messageid;
+            $mua->action = self::MESSAGE_ACTION_READ;
+            $mua->timecreated = $timeread;
+            $mua->id = $DB->insert_record('message_user_actions', $mua);
+
+            // Get the context for the user who received the message.
+            $context = \context_user::instance($userid, IGNORE_MISSING);
+            // If the user no longer exists the context value will be false, in this case use the system context.
+            if ($context === false) {
+                $context = \context_system::instance();
+            }
+
+            // Trigger event for reading a message.
+            $event = \core\event\message_viewed::create(array(
+                'objectid' => $mua->id,
+                'userid' => $userid, // Using the user who read the message as they are the ones performing the action.
+                'context' => $context,
+                'relateduserid' => $message->useridfrom,
+                'other' => array(
+                    'messageid' => $messageid
+                )
+            ));
+            $event->trigger();
+        }
+    }
+
+    /**
+     * Mark a single notification as read.
+     *
+     * @param int $userid The user id who marked the notification as read
+     * @param int $notificationid The notification id
+     * @param int|null $timeread The time the message was marked as read, if null will default to time()
+     */
+    public static function mark_notification_as_read($userid, $notificationid, $timeread = null) {
+        global $DB;
+
+        if (is_null($timeread)) {
+            $timeread = time();
+        }
+
+        // Make sure the notification is for the user.
+        $notification = $DB->get_record('notifications', ['id' => $notificationid, 'useridto' => $userid], '*', MUST_EXIST);
+
+        if (is_null($notification->timeread)) {
+            $updatenotification = new \stdClass();
+            $updatenotification->id = $notification->id;
+            $updatenotification->timeread = $timeread;
+
+            $DB->update_record('notifications', $updatenotification);
+        }
+    }
+
+    /**
+     * Checks if a user can delete a message.
+     *
+     * @param int $userid the user id of who we want to delete the message for (this may be done by the admin
+     *  but will still seem as if it was by the user)
+     * @param int $messageid The message id
+     * @return bool Returns true if a user can delete the message, false otherwise.
+     */
+    public static function can_delete_message($userid, $messageid) {
+        global $DB, $USER;
+
+        $sql = "SELECT m.id, m.useridfrom, mcm.userid as useridto
+                  FROM {messages} m
+            INNER JOIN {message_conversations} mc
+                    ON m.conversationid = mc.id
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = mc.id
+                 WHERE mcm.userid != m.useridfrom
+                   AND m.id = ?";
+        $message = $DB->get_record_sql($sql, [$messageid], MUST_EXIST);
+
+        if ($message->useridfrom == $userid) {
+            $userdeleting = 'useridfrom';
+        } else if ($message->useridto == $userid) {
+            $userdeleting = 'useridto';
+        } else {
+            return false;
+        }
+
+        $systemcontext = \context_system::instance();
+
+        // Let's check if the user is allowed to delete this message.
+        if (has_capability('moodle/site:deleteanymessage', $systemcontext) ||
+            ((has_capability('moodle/site:deleteownmessage', $systemcontext) &&
+                $USER->id == $message->$userdeleting))) {
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Deletes a message.
+     *
+     * This function does not verify any permissions.
+     *
+     * @param int $userid the user id of who we want to delete the message for (this may be done by the admin
+     *  but will still seem as if it was by the user)
+     * @param int $messageid The message id
+     * @return bool
+     */
+    public static function delete_message($userid, $messageid) {
+        global $DB;
+
+        $sql = "SELECT m.id, m.useridfrom, mcm.userid as useridto
+                  FROM {messages} m
+            INNER JOIN {message_conversations} mc
+                    ON m.conversationid = mc.id
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = mc.id
+                 WHERE mcm.userid != m.useridfrom
+                   AND m.id = ?";
+        $message = $DB->get_record_sql($sql, [$messageid], MUST_EXIST);
+
+        // Check if the user has already deleted this message.
+        if (!$DB->record_exists('message_user_actions', ['userid' => $userid,
+                'messageid' => $messageid, 'action' => self::MESSAGE_ACTION_DELETED])) {
+            $mua = new \stdClass();
+            $mua->userid = $userid;
+            $mua->messageid = $messageid;
+            $mua->action = self::MESSAGE_ACTION_DELETED;
+            $mua->timecreated = time();
+            $mua->id = $DB->insert_record('message_user_actions', $mua);
+
+            // Trigger event for deleting a message.
+            \core\event\message_deleted::create_from_ids($message->useridfrom, $message->useridto,
+                $userid, $message->id, $mua->id)->trigger();
+
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Returns the conversation between two users.
+     *
+     * @param int $userid1 The userid of the first user
+     * @param int $userid2 The userid of the second user
+     * @return int|bool The id of the conversation, false if not found
+     */
+    public static function get_conversation_between_users($userid1, $userid2) {
+        global $DB;
+
+        $sql = "SELECT DISTINCT mc.id
+                  FROM {message_conversations} mc
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = mc.id
+            INNER JOIN {message_conversation_members} mcm2
+                    ON mcm2.conversationid = mc.id
+                 WHERE mcm.userid = :userid1
+                   AND mcm2.userid = :userid2
+                   AND mcm.id != mcm2.id";
+
+        if ($conversation = $DB->get_record_sql($sql, ['userid1' => $userid1, 'userid2' => $userid2])) {
+            return $conversation->id;
+        }
+
+        return false;
+    }
+
+    /**
+     * Creates a conversation between two users.
+     *
+     * @param int $userid1 The userid of the first user
+     * @param int $userid2 The userid of the second user
+     * @return int The id of the conversation
+     */
+    public static function create_conversation_between_users($userid1, $userid2) {
+        global $DB;
+
+        $conversation = new \stdClass();
+        $conversation->timecreated = time();
+        $conversation->id = $DB->insert_record('message_conversations', $conversation);
+
+        // Add members to this conversation.
+        $member = new \stdClass();
+        $member->conversationid = $conversation->id;
+        $member->userid = $userid1;
+        $member->timecreated = time();
+        $DB->insert_record('message_conversation_members', $member);
+
+        $member = new \stdClass();
+        $member->conversationid = $conversation->id;
+        $member->userid = $userid2;
+        $member->timecreated = time();
+        $DB->insert_record('message_conversation_members', $member);
+
+        return $conversation->id;
+    }
 }

message/classes/helper.php

@@ -51,46 +51,46 @@ class helper {
                                         $sort = 'timecreated ASC', $timefrom = 0, $timeto = 0) {
         global $DB;
 
-        $messageid = $DB->sql_concat("'message_'", 'id');
+        $sql = "SELECT m.id, m.useridfrom, mdm.userid as useridto, m.subject, m.fullmessage, m.fullmessagehtml,
-        $messagereadid = $DB->sql_concat("'messageread_'", 'id');
+                       m.fullmessageformat, m.smallmessage, m.timecreated, 0 as timeread
+                  FROM {messages} m
+            INNER JOIN {message_conversations} md
+                    ON md.id = m.conversationid
+            INNER JOIN {message_conversation_members} mdm
+                    ON mdm.conversationid = md.id";
+        $params = [];
 
-        $sql = "SELECT {$messageid} AS fakeid, id, useridfrom, useridto, subject, fullmessage, fullmessagehtml, fullmessageformat,
+        if (empty($timedeleted)) {
-                       smallmessage, notification, timecreated, 0 as timeread
+            $sql .= " LEFT JOIN {message_user_actions} mua
-                  FROM {message} m
+                             ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ? AND mua.timecreated is NOT NULL)";
-                 WHERE ((useridto = ? AND useridfrom = ? AND timeusertodeleted = ?)
+            $params[] = $userid;
-                    OR (useridto = ? AND useridfrom = ? AND timeuserfromdeleted = ?))
+            $params[] = api::MESSAGE_ACTION_DELETED;
-                   AND notification = 0
+        } else {
-                   %where%
+            $sql .= " INNER JOIN {message_user_actions} mua
-             UNION ALL
+                              ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ? AND mua.timecreated = ?)";
-                SELECT {$messagereadid} AS fakeid, id, useridfrom, useridto, subject, fullmessage, fullmessagehtml, fullmessageformat,
+            $params[] = $userid;
-                       smallmessage, notification, timecreated, timeread
+            $params[] = api::MESSAGE_ACTION_DELETED;
-                  FROM {message_read} mr
+            $params[] = $timedeleted;
-                 WHERE ((useridto = ? AND useridfrom = ? AND timeusertodeleted = ?)
+        }
-                    OR (useridto = ? AND useridfrom = ? AND timeuserfromdeleted = ?))
-                   AND notification = 0
-                   %where%
-              ORDER BY $sort";
-        $params1 = array($userid, $otheruserid, $timedeleted,
-                         $otheruserid, $userid, $timedeleted);
 
-        $params2 = array($userid, $otheruserid, $timedeleted,
+        $sql .= " WHERE (m.useridfrom = ? AND mdm.userid = ? OR m.useridfrom = ? AND mdm.userid = ?)";
-                         $otheruserid, $userid, $timedeleted);
+        $params = array_merge($params, [$userid, $otheruserid, $otheruserid, $userid]);
-        $where = array();
 
         if (!empty($timefrom)) {
-            $where[] = 'AND timecreated >= ?';
+            $sql .= " AND m.timecreated >= ?";
-            $params1[] = $timefrom;
+            $params[] = $timefrom;
-            $params2[] = $timefrom;
         }
 
         if (!empty($timeto)) {
-            $where[] = 'AND timecreated <= ?';
+            $sql .= " AND m.timecreated <= ?";
-            $params1[] = $timeto;
+            $params[] = $timeto;
-            $params2[] = $timeto;
         }
 
-        $sql = str_replace('%where%', implode(' ', $where), $sql);
+        if (empty($timedeleted)) {
-        $params = array_merge($params1, $params2);
+            $sql .= " AND mua.id is NULL";
+        }
+
+        $sql .= " ORDER BY m.$sort";
 
         return $DB->get_records_sql($sql, $params, $limitfrom, $limitnum);
     }

message/classes/search/base_message.php

@@ -145,9 +145,15 @@ abstract class base_message extends \core_search\base {
             $userfield) {
         global $DB;
 
+        if ($userfield == 'useridto') {
+            $userfield = 'mcm.userid';
+        } else {
+            $userfield = 'm.useridfrom';
+        }
+
         // Set up basic query.
         $where = $userfield . ' != :noreplyuser AND ' . $userfield .
-                ' != :supportuser AND timecreated >= :modifiedfrom';
+                ' != :supportuser AND m.timecreated >= :modifiedfrom';
         $params = [
             'noreplyuser' => \core_user::NOREPLY_USER,
             'supportuser' => \core_user::SUPPORT_USER,
@@ -179,6 +185,15 @@ abstract class base_message extends \core_search\base {
                 throw new \coding_exception('Unexpected contextlevel: ' . $context->contextlevel);
         }
 
-        return $DB->get_recordset_select('message_read', $where, $params, 'timeread ASC');
+        $sql = "SELECT m.*, mcm.userid as useridto
+                  FROM {messages} m
+            INNER JOIN {message_conversations} mc
+                    ON m.conversationid = mc.id
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = mc.id
+                 WHERE mcm.userid != m.useridfrom
+                   AND $where
+              ORDER BY m.timecreated ASC";
+        return $DB->get_recordset_sql($sql, $params);
     }
 }

message/classes/search/message_received.php

@@ -70,7 +70,15 @@ class message_received extends base_message {
             return \core_search\manager::ACCESS_DENIED;
         }
 
-        $message = $DB->get_record('message_read', array('id' => $id));
+        $sql = "SELECT m.*, mcm.userid as useridto
+                  FROM {messages} m
+            INNER JOIN {message_conversations} mc
+                    ON m.conversationid = mc.id
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = mc.id
+                 WHERE mcm.userid != m.useridfrom
+                   AND m.id = :id";
+        $message = $DB->get_record_sql($sql, array('id' => $id));
         if (!$message) {
             return \core_search\manager::ACCESS_DELETED;
         }
@@ -86,7 +94,9 @@ class message_received extends base_message {
             return \core_search\manager::ACCESS_DENIED;
         }
 
-        if ($message->timeusertodeleted != 0) {
+        $usertodeleted = $DB->record_exists('message_user_actions', ['messageid' => $id, 'userid' => $message->useridto,
+            'action' => \core_message\api::MESSAGE_ACTION_DELETED]);
+        if ($usertodeleted) {
             return \core_search\manager::ACCESS_DELETED;
         }
 

message/classes/search/message_sent.php

@@ -69,7 +69,15 @@ class message_sent extends base_message {
             return \core_search\manager::ACCESS_DENIED;
         }
 
-        $message = $DB->get_record('message_read', array('id' => $id));
+        $sql = "SELECT m.*, mcm.userid as useridto
+                  FROM {messages} m
+            INNER JOIN {message_conversations} mc
+                    ON m.conversationid = mc.id
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = mc.id
+                 WHERE mcm.userid != m.useridfrom
+                   AND m.id = :id";
+        $message = $DB->get_record_sql($sql, array('id' => $id));
         if (!$message) {
             return \core_search\manager::ACCESS_DELETED;
         }
@@ -85,7 +93,9 @@ class message_sent extends base_message {
             return \core_search\manager::ACCESS_DENIED;
         }
 
-        if ($message->timeuserfromdeleted != 0) {
+        $userfromdeleted = $DB->record_exists('message_user_actions', ['messageid' => $id, 'userid' => $message->useridfrom,
+            'action' => \core_message\api::MESSAGE_ACTION_DELETED]);
+        if ($userfromdeleted) {
             return \core_search\manager::ACCESS_DELETED;
         }
 

message/externallib.php

@@ -1161,7 +1161,7 @@ class core_message_external extends external_api {
      * @since Moodle 2.5
      */
     public static function get_contacts() {
-        global $CFG, $PAGE;
+        global $CFG, $PAGE, $USER;
 
         // Check if messaging is enabled.
         if (empty($CFG->messaging)) {
@@ -1170,25 +1170,68 @@ class core_message_external extends external_api {
 
         require_once($CFG->dirroot . '/user/lib.php');
 
-        list($online, $offline, $strangers) = message_get_contacts();
+        $allcontacts = array('online' => [], 'offline' => [], 'strangers' => []);
-        $allcontacts = array('online' => $online, 'offline' => $offline, 'strangers' => $strangers);
+        $contacts = \core_message\api::get_contacts_with_unread_message_count($USER->id);
-        foreach ($allcontacts as $mode => $contacts) {
+        foreach ($contacts as $contact) {
-            foreach ($contacts as $key => $contact) {
+            // Set the mode.
-                $newcontact = array(
+            $mode = 'offline';
-                    'id' => $contact->id,
+            if (\core_message\helper::is_online($contact->lastaccess)) {
-                    'fullname' => fullname($contact),
+                $mode = 'online';
-                    'unread' => $contact->messagecount
+            }
-                );
 
-                $userpicture = new user_picture($contact);
+            $newcontact = array(
-                $userpicture->size = 1; // Size f1.
+                'id' => $contact->id,
-                $newcontact['profileimageurl'] = $userpicture->get_url($PAGE)->out(false);
+                'fullname' => fullname($contact),
-                $userpicture->size = 0; // Size f2.
+                'unread' => $contact->messagecount
-                $newcontact['profileimageurlsmall'] = $userpicture->get_url($PAGE)->out(false);
+            );
 
-                $allcontacts[$mode][$key] = $newcontact;
+            $userpicture = new user_picture($contact);
+            $userpicture->size = 1; // Size f1.
+            $newcontact['profileimageurl'] = $userpicture->get_url($PAGE)->out(false);
+            $userpicture->size = 0; // Size f2.
+            $newcontact['profileimageurlsmall'] = $userpicture->get_url($PAGE)->out(false);
+
+            $allcontacts[$mode][$contact->id] = $newcontact;
+        }
+
+        $strangers = \core_message\api::get_non_contacts_with_unread_message_count($USER->id);
+        foreach ($strangers as $contact) {
+            $newcontact = array(
+                'id' => $contact->id,
+                'fullname' => fullname($contact),
+                'unread' => $contact->messagecount
+            );
+
+            $userpicture = new user_picture($contact);
+            $userpicture->size = 1; // Size f1.
+            $newcontact['profileimageurl'] = $userpicture->get_url($PAGE)->out(false);
+            $userpicture->size = 0; // Size f2.
+            $newcontact['profileimageurlsmall'] = $userpicture->get_url($PAGE)->out(false);
+
+            $allcontacts['strangers'][$contact->id] = $newcontact;
+        }
+
+        // Add noreply user and support user to the list, if they don't exist.
+        $supportuser = core_user::get_support_user();
+        if (!isset($strangers[$supportuser->id]) && !$supportuser->deleted) {
+            $supportuser->messagecount = message_count_unread_messages($USER, $supportuser);
+            if ($supportuser->messagecount > 0) {
+                $supportuser->fullname = fullname($supportuser);
+                $supportuser->unread = $supportuser->messagecount;
+                $allcontacts['strangers'][$supportuser->id] = $supportuser;
             }
         }
+
+        $noreplyuser = core_user::get_noreply_user();
+        if (!isset($strangers[$noreplyuser->id]) && !$noreplyuser->deleted) {
+            $noreplyuser->messagecount = message_count_unread_messages($USER, $noreplyuser);
+            if ($noreplyuser->messagecount > 0) {
+                $noreplyuser->fullname = fullname($noreplyuser);
+                $noreplyuser->unread = $noreplyuser->messagecount;
+                $allcontacts['strangers'][$noreplyuser->id] = $noreplyuser;
+            }
+        }
+
         return $allcontacts;
     }
 
@@ -1486,13 +1529,16 @@ class core_message_external extends external_api {
             foreach ($messages as $mid => $message) {
 
                 // Do not return deleted messages.
-                if (($useridto == $USER->id and $message->timeusertodeleted) or
+                if (!$message->notification) {
+                    if (($useridto == $USER->id and $message->timeusertodeleted) or
                         ($useridfrom == $USER->id and $message->timeuserfromdeleted)) {
-
+                        unset($messages[$mid]);
-                    unset($messages[$mid]);
+                        continue;
-                    continue;
+                    }
                 }
 
+                $message->useridto = $useridto;
+
                 // We need to get the user from the query.
                 if (empty($userfromfullname)) {
                     // Check for non-reply and support users.
@@ -1517,11 +1563,6 @@ class core_message_external extends external_api {
                     $message->usertofullname = $usertofullname;
                 }
 
-                // This field is only available in the message_read table.
-                if (!isset($message->timeread)) {
-                    $message->timeread = 0;
-                }
-
                 $message->text = message_format_message_text($message);
                 $messages[$mid] = (array) $message;
             }
@@ -1774,7 +1815,7 @@ class core_message_external extends external_api {
         }
 
         // Now, we can get safely all the blocked users.
-        $users = message_get_blocked_users($user);
+        $users = \core_message\api::get_blocked_users($user->id);
 
         $blockedusers = array();
         foreach ($users as $user) {
@@ -1875,16 +1916,24 @@ class core_message_external extends external_api {
         $context = context_system::instance();
         self::validate_context($context);
 
-        $message = $DB->get_record('message', array('id' => $params['messageid']), '*', MUST_EXIST);
+        $sql = "SELECT m.*, mcm.userid as useridto
+                  FROM {messages} m
+            INNER JOIN {message_conversations} mc
+                    ON m.conversationid = mc.id
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = mc.id
+                 WHERE mcm.userid != m.useridfrom
+                   AND m.id = ?";
+        $message = $DB->get_record_sql($sql, [$params['messageid']], MUST_EXIST);
 
         if ($message->useridto != $USER->id) {
             throw new invalid_parameter_exception('Invalid messageid, you don\'t have permissions to mark this message as read');
         }
 
-        $messageid = message_mark_message_read($message, $timeread);
+        \core_message\api::mark_message_as_read($USER->id, $message->id, $timeread);
 
         $results = array(
-            'messageid' => $messageid,
+            'messageid' => $message->id,
             'warnings' => $warnings
         );
         return $results;
@@ -2094,7 +2143,7 @@ class core_message_external extends external_api {
      * @since 3.1
      */
     public static function delete_message($messageid, $userid, $read = true) {
-        global $CFG, $DB;
+        global $CFG;
 
         // Check if private messaging between users is allowed.
         if (empty($CFG->messaging)) {
@@ -2116,15 +2165,11 @@ class core_message_external extends external_api {
         $context = context_system::instance();
         self::validate_context($context);
 
-        $messagestable = $params['read'] ? 'message_read' : 'message';
-        $message = $DB->get_record($messagestable, array('id' => $params['messageid']), '*', MUST_EXIST);
-
         $user = core_user::get_user($params['userid'], '*', MUST_EXIST);
         core_user::require_active_user($user);
 
-        $status = false;
+        if (\core_message\api::can_delete_message($user->id, $messageid)) {
-        if (message_can_delete_message($message, $user->id)) {
+            $status = \core_message\api::delete_message($user->id, $messageid);
-            $status = message_delete_message($message, $user->id);;
         } else {
             throw new moodle_exception('You do not have permission to delete this message');
         }

message/lib.php

@@ -64,147 +64,6 @@ define('MESSAGE_DEFAULT_MIN_POLL_IN_SECONDS', 10);
 define('MESSAGE_DEFAULT_MAX_POLL_IN_SECONDS', 2 * MINSECS);
 define('MESSAGE_DEFAULT_TIMEOUT_POLL_IN_SECONDS', 5 * MINSECS);
 
-/**
- * Retrieve users blocked by $user1
- *
- * @param object $user1 the user whose messages are being viewed
- * @param object $user2 the user $user1 is talking to. If they are being blocked
- *                      they will have a variable called 'isblocked' added to their user object
- * @return array the users blocked by $user1
- */
-function message_get_blocked_users($user1=null, $user2=null) {
-    global $DB, $USER;
-
-    if (empty($user1)) {
-        $user1 = $USER;
-    }
-
-    if (!empty($user2)) {
-        $user2->isblocked = false;
-    }
-
-    $blockedusers = array();
-
-    $userfields = user_picture::fields('u', array('lastaccess'));
-    $blockeduserssql = "SELECT $userfields, COUNT(m.id) AS messagecount
-                          FROM {message_contacts} mc
-                          JOIN {user} u ON u.id = mc.contactid
-                          LEFT OUTER JOIN {message} m ON m.useridfrom = mc.contactid AND m.useridto = :user1id1
-                         WHERE u.deleted = 0 AND mc.userid = :user1id2 AND mc.blocked = 1
-                      GROUP BY $userfields
-                      ORDER BY u.firstname ASC";
-    $rs =  $DB->get_recordset_sql($blockeduserssql, array('user1id1' => $user1->id, 'user1id2' => $user1->id));
-
-    foreach($rs as $rd) {
-        $blockedusers[] = $rd;
-
-        if (!empty($user2) && $user2->id == $rd->id) {
-            $user2->isblocked = true;
-        }
-    }
-    $rs->close();
-
-    return $blockedusers;
-}
-
-/**
- * Retrieve $user1's contacts (online, offline and strangers)
- *
- * @param object $user1 the user whose messages are being viewed
- * @param object $user2 the user $user1 is talking to. If they are a contact
- *                      they will have a variable called 'iscontact' added to their user object
- * @return array containing 3 arrays. array($onlinecontacts, $offlinecontacts, $strangers)
- */
-function message_get_contacts($user1=null, $user2=null) {
-    global $DB, $CFG, $USER;
-
-    if (empty($user1)) {
-        $user1 = $USER;
-    }
-
-    if (!empty($user2)) {
-        $user2->iscontact = false;
-    }
-
-    $timetoshowusers = 300; //Seconds default
-    if (isset($CFG->block_online_users_timetosee)) {
-        $timetoshowusers = $CFG->block_online_users_timetosee * 60;
-    }
-
-    // time which a user is counting as being active since
-    $timefrom = time()-$timetoshowusers;
-
-    // people in our contactlist who are online
-    $onlinecontacts  = array();
-    // people in our contactlist who are offline
-    $offlinecontacts = array();
-    // people who are not in our contactlist but have sent us a message
-    $strangers       = array();
-
-    $userfields = user_picture::fields('u', array('lastaccess'));
-
-    // get all in our contactlist who are not blocked in our contact list
-    // and count messages we have waiting from each of them
-    $contactsql = "SELECT $userfields, COUNT(m.id) AS messagecount
-                     FROM {message_contacts} mc
-                     JOIN {user} u ON u.id = mc.contactid
-                     LEFT OUTER JOIN {message} m ON m.useridfrom = mc.contactid AND m.useridto = ?
-                    WHERE u.deleted = 0 AND mc.userid = ? AND mc.blocked = 0
-                 GROUP BY $userfields
-                 ORDER BY u.firstname ASC";
-
-    $rs = $DB->get_recordset_sql($contactsql, array($user1->id, $user1->id));
-    foreach ($rs as $rd) {
-        if ($rd->lastaccess >= $timefrom) {
-            // they have been active recently, so are counted online
-            $onlinecontacts[] = $rd;
-
-        } else {
-            $offlinecontacts[] = $rd;
-        }
-
-        if (!empty($user2) && $user2->id == $rd->id) {
-            $user2->iscontact = true;
-        }
-    }
-    $rs->close();
-
-    // get messages from anyone who isn't in our contact list and count the number
-    // of messages we have from each of them
-    $strangersql = "SELECT $userfields, count(m.id) as messagecount
-                      FROM {message} m
-                      JOIN {user} u  ON u.id = m.useridfrom
-                      LEFT OUTER JOIN {message_contacts} mc ON mc.contactid = m.useridfrom AND mc.userid = m.useridto
-                     WHERE u.deleted = 0 AND mc.id IS NULL AND m.useridto = ?
-                  GROUP BY $userfields
-                  ORDER BY u.firstname ASC";
-
-    $rs = $DB->get_recordset_sql($strangersql, array($USER->id));
-    // Add user id as array index, so supportuser and noreply user don't get duplicated (if they are real users).
-    foreach ($rs as $rd) {
-        $strangers[$rd->id] = $rd;
-    }
-    $rs->close();
-
-    // Add noreply user and support user to the list, if they don't exist.
-    $supportuser = core_user::get_support_user();
-    if (!isset($strangers[$supportuser->id])) {
-        $supportuser->messagecount = message_count_unread_messages($USER, $supportuser);
-        if ($supportuser->messagecount > 0) {
-            $strangers[$supportuser->id] = $supportuser;
-        }
-    }
-
-    $noreplyuser = core_user::get_noreply_user();
-    if (!isset($strangers[$noreplyuser->id])) {
-        $noreplyuser->messagecount = message_count_unread_messages($USER, $noreplyuser);
-        if ($noreplyuser->messagecount > 0) {
-            $strangers[$noreplyuser->id] = $noreplyuser;
-        }
-    }
-    return array($onlinecontacts, $offlinecontacts, $strangers);
-}
-
 /**
  * Returns the count of unread messages for user. Either from a specific user or from all users.
  *
@@ -219,15 +78,24 @@ function message_count_unread_messages($user1=null, $user2=null) {
         $user1 = $USER;
     }
 
+    $sql = "SELECT COUNT(m.id)
+              FROM {messages} m
+        INNER JOIN {message_conversations} mc
+                ON mc.id = m.conversationid
+        INNER JOIN {message_conversation_members} mcm
+                ON mcm.conversationid = mc.id
+         LEFT JOIN {message_user_actions} mua
+                ON (mua.messageid = m.id AND mua.userid = ? AND (mua.action = ? OR mua.action = ?))
+             WHERE mua.id is NULL
+               AND mcm.userid = ?";
+    $params = [$user1->id, \core_message\api::MESSAGE_ACTION_DELETED, \core_message\api::MESSAGE_ACTION_READ,  $user1->id];
+
     if (!empty($user2)) {
-        return $DB->count_records_select('message', "useridto = ? AND useridfrom = ? AND notification = 0
+        $sql .= " AND m.useridfrom = ?";
-            AND timeusertodeleted = 0",
+        $params[] = $user2->id;
-            array($user1->id, $user2->id), "COUNT('id')");
-    } else {
-        return $DB->count_records_select('message', "useridto = ? AND notification = 0
-            AND timeusertodeleted = 0",
-            array($user1->id), "COUNT('id')");
     }
+
+    return $DB->count_records_sql($sql, $params);
 }
 
 /**
@@ -249,7 +117,7 @@ function message_format_message_text($message, $forcetexttohtml = false) {
     $format = $message->fullmessageformat;
 
     if (strval($message->smallmessage) !== '') {
-        if ($message->notification == 1) {
+        if (!empty($message->notification)) {
             if (strval($message->fullmessagehtml) !== '' or strval($message->fullmessage) !== '') {
                 $format = FORMAT_PLAIN;
             }
@@ -421,86 +289,6 @@ function message_block_contact($contactid, $userid = 0) {
     return message_add_contact($contactid, 1, $userid);
 }
 
-/**
- * Checks if a user can delete a message.
- *
- * @param stdClass $message the message to delete
- * @param string $userid the user id of who we want to delete the message for (this may be done by the admin
- *  but will still seem as if it was by the user)
- * @return bool Returns true if a user can delete the message, false otherwise.
- */
-function message_can_delete_message($message, $userid) {
-    global $USER;
-
-    if ($message->useridfrom == $userid) {
-        $userdeleting = 'useridfrom';
-    } else if ($message->useridto == $userid) {
-        $userdeleting = 'useridto';
-    } else {
-        return false;
-    }
-
-    $systemcontext = context_system::instance();
-
-    // Let's check if the user is allowed to delete this message.
-    if (has_capability('moodle/site:deleteanymessage', $systemcontext) ||
-        ((has_capability('moodle/site:deleteownmessage', $systemcontext) &&
-            $USER->id == $message->$userdeleting))) {
-        return true;
-    }
-
-    return false;
-}
-
-/**
- * Deletes a message.
- *
- * This function does not verify any permissions.
- *
- * @param stdClass $message the message to delete
- * @param string $userid the user id of who we want to delete the message for (this may be done by the admin
- *  but will still seem as if it was by the user)
- * @return bool
- */
-function message_delete_message($message, $userid) {
-    global $DB;
-
-    // The column we want to alter.
-    if ($message->useridfrom == $userid) {
-        $coltimedeleted = 'timeuserfromdeleted';
-    } else if ($message->useridto == $userid) {
-        $coltimedeleted = 'timeusertodeleted';
-    } else {
-        return false;
-    }
-
-    // Don't update it if it's already been deleted.
-    if ($message->$coltimedeleted > 0) {
-        return false;
-    }
-
-    // Get the table we want to update.
-    if (isset($message->timeread)) {
-        $messagetable = 'message_read';
-    } else {
-        $messagetable = 'message';
-    }
-
-    // Mark the message as deleted.
-    $updatemessage = new stdClass();
-    $updatemessage->id = $message->id;
-    $updatemessage->$coltimedeleted = time();
-    $success = $DB->update_record($messagetable, $updatemessage);
-
-    if ($success) {
-        // Trigger event for deleting a message.
-        \core\event\message_deleted::create_from_ids($message->useridfrom, $message->useridto,
-            $userid, $messagetable, $message->id)->trigger();
-    }
-
-    return $success;
-}
-
 /**
  * Load a user's contact record
  *
@@ -722,71 +510,6 @@ function message_post_message($userfrom, $userto, $message, $format) {
     return message_send($eventdata);
 }
 
-/**
- * Moves messages from a particular user from the message table (unread messages) to message_read
- * This is typically only used when a user is deleted
- *
- * @param object $userid User id
- * @return boolean success
- */
-function message_move_userfrom_unread2read($userid) {
-    global $DB;
-
-    // move all unread messages from message table to message_read
-    if ($messages = $DB->get_records_select('message', 'useridfrom = ?', array($userid), 'timecreated')) {
-        foreach ($messages as $message) {
-            message_mark_message_read($message, 0); //set timeread to 0 as the message was never read
-        }
-    }
-    return true;
-}
-
-/**
- * Mark a single message as read
- *
- * @param stdClass $message An object with an object property ie $message->id which is an id in the message table
- * @param int $timeread the timestamp for when the message should be marked read. Usually time().
- * @param bool $messageworkingempty Is the message_working table already confirmed empty for this message?
- * @return int the ID of the message in the message_read table
- */
-function message_mark_message_read($message, $timeread, $messageworkingempty=false) {
-    global $DB;
-
-    $message->timeread = $timeread;
-
-    $messageid = $message->id;
-    unset($message->id);//unset because it will get a new id on insert into message_read
-
-    //If any processors have pending actions abort them
-    if (!$messageworkingempty) {
-        $DB->delete_records('message_working', array('unreadmessageid' => $messageid));
-    }
-    $messagereadid = $DB->insert_record('message_read', $message);
-
-    $DB->delete_records('message', array('id' => $messageid));
-
-    // Get the context for the user who received the message.
-    $context = context_user::instance($message->useridto, IGNORE_MISSING);
-    // If the user no longer exists the context value will be false, in this case use the system context.
-    if ($context === false) {
-        $context = context_system::instance();
-    }
-
-    // Trigger event for reading a message.
-    $event = \core\event\message_viewed::create(array(
-        'objectid' => $messagereadid,
-        'userid' => $message->useridto, // Using the user who read the message as they are the ones performing the action.
-        'context' => $context,
-        'relateduserid' => $message->useridfrom,
-        'other' => array(
-            'messageid' => $messageid
-        )
-    ));
-    $event->trigger();
-
-    return $messagereadid;
-}
-
 /**
  * Get all message processors, validate corresponding plugin existance and
  * system configuration
@@ -962,42 +685,108 @@ function message_get_messages($useridto, $useridfrom = 0, $notifications = -1, $
                                 $sort = 'mr.timecreated DESC', $limitfrom = 0, $limitnum = 0) {
     global $DB;
 
-    $messagetable = $read ? '{message_read}' : '{message}';
+    // If the 'useridto' value is empty then we are going to retrieve messages sent by the useridfrom to any user.
-    $params = array('deleted' => 0);
-
-    // Empty useridto means that we are going to retrieve messages send by the useridfrom to any user.
     if (empty($useridto)) {
         $userfields = get_all_user_name_fields(true, 'u', '', 'userto');
-        $joinsql = "JOIN {user} u ON u.id = mr.useridto";
-        $usersql = "mr.useridfrom = :useridfrom AND u.deleted = :deleted";
-        $params['useridfrom'] = $useridfrom;
     } else {
         $userfields = get_all_user_name_fields(true, 'u', '', 'userfrom');
+    }
+
+    // Create the SQL we will be using.
+    $messagesql = "SELECT mr.*, $userfields, 0 as notification, '' as contexturl, '' as contexturlname,
+                          mua.timecreated as timeusertodeleted, mua2.timecreated as timeread,
+                          mua3.timecreated as timeuserfromdeleted
+                     FROM {messages} mr
+               INNER JOIN {message_conversations} mc
+                       ON mc.id = mr.conversationid
+               INNER JOIN {message_conversation_members} mcm
+                       ON mcm.conversationid = mc.id ";
+
+    $notificationsql = "SELECT mr.*, $userfields, 1 as notification
+                          FROM {notifications} mr ";
+
+    $messagejoinsql = "LEFT JOIN {message_user_actions} mua
+                              ON (mua.messageid = mr.id AND mua.userid = mcm.userid AND mua.action = ?)
+                       LEFT JOIN {message_user_actions} mua2
+                              ON (mua2.messageid = mr.id AND mua2.userid = mcm.userid AND mua2.action = ?)
+                       LEFT JOIN {message_user_actions} mua3
+                              ON (mua3.messageid = mr.id AND mua3.userid = mr.useridfrom AND mua3.action = ?)";
+    $messagejoinparams = [\core_message\api::MESSAGE_ACTION_DELETED, \core_message\api::MESSAGE_ACTION_READ,
+        \core_message\api::MESSAGE_ACTION_DELETED];
+    $notificationsparams = [];
+
+    // If the 'useridto' value is empty then we are going to retrieve messages sent by the useridfrom to any user.
+    if (empty($useridto)) {
+        // Create the messaging query and params.
+        $messagesql .= "INNER JOIN {user} u
+                                ON u.id = mcm.userid
+                                $messagejoinsql
+                             WHERE mr.useridfrom = ?
+                               AND mr.useridfrom != mcm.userid
+                               AND u.deleted = 0 ";
+        $messageparams = array_merge($messagejoinparams, [$useridfrom]);
+
+        // Create the notifications query and params.
+        $notificationsql .= "INNER JOIN {user} u
+                                     ON u.id = mr.useridto
+                                  WHERE mr.useridfrom = ?
+                                    AND u.deleted = 0 ";
+        $notificationsparams[] = $useridfrom;
+    } else {
+        // Create the messaging query and params.
         // Left join because useridfrom may be -10 or -20 (no-reply and support users).
-        $joinsql = "LEFT JOIN {user} u ON u.id = mr.useridfrom";
+        $messagesql .= "LEFT JOIN {user} u
-        $usersql = "mr.useridto = :useridto AND (u.deleted IS NULL OR u.deleted = :deleted)";
+                               ON u.id = mr.useridfrom
-        $params['useridto'] = $useridto;
+                               $messagejoinsql
+                            WHERE mcm.userid = ?
+                              AND mr.useridfrom != mcm.userid
+                              AND u.deleted = 0 ";
+        $messageparams = array_merge($messagejoinparams, [$useridto]);
         if (!empty($useridfrom)) {
-            $usersql .= " AND mr.useridfrom = :useridfrom";
+            $messagesql .= " AND mr.useridfrom = ? ";
-            $params['useridfrom'] = $useridfrom;
+            $messageparams[] = $useridfrom;
+        }
+
+        // Create the notifications query and params.
+        // Left join because useridfrom may be -10 or -20 (no-reply and support users).
+        $notificationsql .= "LEFT JOIN {user} u
+                                    ON (u.id = mr.useridfrom AND u.deleted = 0)
+                                 WHERE mr.useridto = ? ";
+        $notificationsparams[] = $useridto;
+        if (!empty($useridfrom)) {
+            $notificationsql .= " AND mr.useridfrom = ? ";
+            $notificationsparams[] = $useridfrom;
         }
     }
+    if ($read) {
+        $notificationsql .= "AND mr.timeread IS NOT NULL ";
+    } else {
+        $notificationsql .= "AND mr.timeread IS NULL ";
+    }
+    $messagesql .= "ORDER BY $sort";
+    $notificationsql .= "ORDER BY $sort";
 
-    // Now, if retrieve notifications, conversations or both.
+    // Handle messages if needed.
-    $typesql = "";
+    if ($notifications === -1 || $notifications === 0) {
-    if ($notifications !== -1) {
+        $messages = $DB->get_records_sql($messagesql, $messageparams, $limitfrom, $limitnum);
-        $typesql = "AND mr.notification = :notification";
+        // Get rid of the messages that have either been read or not read depending on the value of $read.
-        $params['notification'] = ($notifications) ? 1 : 0;
+        $messages = array_filter($messages, function ($message) use ($read) {
+            if ($read) {
+                return !is_null($message->timeread);
+            }
+
+            return is_null($message->timeread);
+        });
     }
 
-    $sql = "SELECT mr.*, $userfields
+    // All.
-              FROM $messagetable mr
+    if ($notifications === -1) {
-                   $joinsql
+        return array_merge($messages, $DB->get_records_sql($notificationsql, $notificationsparams, $limitfrom, $limitnum));
-             WHERE $usersql
+    } else if ($notifications === 1) { // Just notifications.
-                   $typesql
+        return $DB->get_records_sql($notificationsql, $notificationsparams, $limitfrom, $limitnum);
-             ORDER BY $sort";
+    }
 
-    $messages = $DB->get_records_sql($sql, $params, $limitfrom, $limitnum);
+    // Just messages.
     return $messages;
 }
 

message/output/popup/classes/api.php

@@ -69,34 +69,21 @@ class api {
             $disabled = $user->emailstop;
         }
         if ($disabled) {
-            // Notifications are disabled, no need to run giant queries.
+            // Notifications are disabled.
             return array();
         }
 
-        $sql = "SELECT r.id, r.useridfrom, r.useridto,
+        $sql = "SELECT n.id, n.useridfrom, n.useridto,
-                       r.subject, r.fullmessage, r.fullmessageformat,
+                       n.subject, n.fullmessage, n.fullmessageformat,
-                       r.fullmessagehtml, r.smallmessage, r.notification, r.contexturl,
+                       n.fullmessagehtml, n.smallmessage, n.contexturl,
-                       r.contexturlname, r.timecreated, r.timeuserfromdeleted, r.timeusertodeleted,
+                       n.contexturlname, n.timecreated, n.component,
-                       r.component, r.eventtype, r.timeread
+                       n.eventtype, n.timeread
-                  FROM {message_read} r
+                  FROM {notifications} n
-                 WHERE r.notification = 1
+                 WHERE n.id IN (SELECT messageid FROM {message_popup})
-                       AND r.id IN (SELECT messageid FROM {message_popup} WHERE isread = 1)
+                   AND n.useridto = :useridto1
-                       AND r.useridto = :useridto1
-             UNION ALL
-                SELECT u.id, u.useridfrom, u.useridto,
-                       u.subject, u.fullmessage, u.fullmessageformat,
-                       u.fullmessagehtml, u.smallmessage, u.notification, u.contexturl,
-                       u.contexturlname, u.timecreated, u.timeuserfromdeleted, u.timeusertodeleted,
-                       u.component, u.eventtype, 0 as timeread
-                  FROM {message} u
-                 WHERE u.notification = 1
-                       AND u.id IN (SELECT messageid FROM {message_popup} WHERE isread = 0)
-                       AND u.useridto = :useridto2
               ORDER BY timecreated $sort, timeread $sort, id $sort";
 
         $notifications = [];
-        // Use recordset here to ensure records with the same id aren't ignored because
-        // we can have id clashes between the message and message_read tables.
         $records = $DB->get_recordset_sql($sql, $params, $offset, $limit);
         foreach ($records as $record) {
             $notifications[] = (object) $record;
@@ -122,9 +109,9 @@ class api {
 
         return $DB->count_records_sql(
             "SELECT count(id)
-            FROM {message}
+               FROM {notifications}
-            WHERE id IN (SELECT messageid FROM {message_popup} WHERE isread = 0)
+              WHERE id IN (SELECT messageid FROM {message_popup} WHERE isread = 0)
-            AND useridto = ?",
+                AND useridto = ?",
             [$useridto]
         );
     }

message/output/popup/classes/output/popup_notification.php

@@ -57,16 +57,7 @@ class popup_notification implements templatable, renderable {
     }
 
     public function export_for_template(\renderer_base $output) {
-        global $USER;
-
         $context = clone $this->notification;
-
-        if ($context->useridto == $USER->id && $context->timeusertodeleted) {
-            $context->deleted = true;
-        } else {
-            $context->deleted = false;
-        }
-
         $context->timecreatedpretty = get_string('ago', 'message', format_time(time() - $context->timecreated));
         $context->text = message_format_message_text($context);
         $context->read = $context->timeread ? true : false;

message/output/popup/externallib.php

@@ -117,6 +117,9 @@ class message_popup_external extends external_api {
                 $notificationoutput = new \message_popup\output\popup_notification($notification);
 
                 $notificationcontext = $notificationoutput->export_for_template($renderer);
+
+                // Keep this for BC.
+                $notificationcontext->deleted = false;
                 $notificationcontexts[] = $notificationcontext;
             }
         }

message/output/popup/message_output_popup.php

@@ -55,6 +55,7 @@ class message_output_popup extends message_output {
             $procmessage = new stdClass();
             $procmessage->unreadmessageid = $eventdata->savedmessageid;
             $procmessage->processorid     = $processorid;
+            $procmessage->notification    = $eventdata->notification;
 
             //save this message for later delivery
             $DB->insert_record('message_working', $procmessage);
@@ -122,7 +123,6 @@ class message_output_popup extends message_output {
         global $DB;
 
         if ($record = $DB->get_record('message_popup', ['messageid' => $event->other['messageid']])) {
-            // The id can change when the moving to the message_read table.
             $record->messageid = $event->objectid;
             $record->isread = 1;
             $DB->update_record('message_popup', $record);

message/output/popup/tests/base.php

@@ -50,7 +50,7 @@ trait message_popup_test_helper {
         $record->smallmessage = $message;
         $record->timecreated = $timecreated ? $timecreated : time();
 
-        $id = $DB->insert_record('message', $record);
+        $id = $DB->insert_record('notifications', $record);
 
         $popup = new stdClass();
         $popup->messageid = $id;
@@ -89,7 +89,10 @@ trait message_popup_test_helper {
         $record->timecreated = $timecreated ? $timecreated : time();
         $record->timeread = $timeread ? $timeread : time();
 
-        $id = $DB->insert_record('message_read', $record);
+        $id = $DB->insert_record('notifications', $record);
+
+        // Mark it as read.
+        \core_message\api::mark_notification_as_read($userto->id, $id);
 
         $popup = new stdClass();
         $popup->messageid = $id;

message/output/popup/tests/behat/message_popover_unread.feature

@@ -13,7 +13,7 @@ Feature: Message popover unread messages
     And I send "Test message" message to "Student 1" user
     And I log out
 
-  Scenario: Notification popover shows correct unread count
+  Scenario: Message popover shows correct unread count
     When I log in as "student2"
     And I send "Test message 2" message to "Student 1" user
     And I log out
@@ -38,7 +38,7 @@ Feature: Message popover unread messages
     # Confirm the message was loaded in the messaging page.
     And I should see "Test message" in the "[data-region='message-text']" "css_element"
 
-  Scenario: Mark all notifications as read
+  Scenario: Mark all messages as read
     When I log in as "student1"
     # Open the popover.
     And I open the message popover

message/tests/api_test.php

@@ -631,7 +631,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
                             'messageposition'   => 0,
                             'with'              => 'user1',
                             'subject'           => 'S2',
-                            'unreadcount'       => 2,
+                            'unreadcount'       => 0, // Messages sent to and from the same user are counted as read.
                         ),
                     ),
                 ),
@@ -872,11 +872,9 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
             $subject    = $messagedata['subject'];
 
             if (isset($messagedata['state']) && $messagedata['state'] == 'unread') {
-                $table = 'message';
                 $messageid = $this->send_fake_message($from, $to, $subject);
             } else {
                 // If there is no state, or the state is not 'unread', assume the message is read.
-                $table = 'message_read';
                 $messageid = message_post_message($from, $to, $subject, FORMAT_PLAIN);
             }
 
@@ -890,7 +888,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
                 $updatemessage->timecreated = $defaulttimecreated;
             }
 
-            $DB->update_record($table, $updatemessage);
+            $DB->update_record('messages', $updatemessage);
         }
 
         foreach ($expectations as $username => $data) {
@@ -1155,34 +1153,39 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
 
         // Send some messages back and forth.
         $time = 1;
-        $this->send_fake_message($user1, $user2, 'Yo!', 0, $time + 1);
+        $m1id = $this->send_fake_message($user1, $user2, 'Yo!', 0, $time + 1);
-        $this->send_fake_message($user2, $user1, 'Sup mang?', 0, $time + 2);
+        $m2id = $this->send_fake_message($user2, $user1, 'Sup mang?', 0, $time + 2);
-        $this->send_fake_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 3);
+        $m3id = $this->send_fake_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 3);
-        $this->send_fake_message($user2, $user1, 'Word.', 0, $time + 4);
+        $m4id = $this->send_fake_message($user2, $user1, 'Word.', 0, $time + 4);
 
         // Delete the conversation as user 1.
         \core_message\api::delete_conversation($user1->id, $user2->id);
 
-        $messages = $DB->get_records('message', array(), 'timecreated ASC');
+        $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC');
-        $this->assertCount(4, $messages);
+        $this->assertCount(4, $muas);
+        // Sort by id.
+        ksort($muas);
 
-        $message1 = array_shift($messages);
+        $mua1 = array_shift($muas);
-        $message2 = array_shift($messages);
+        $mua2 = array_shift($muas);
-        $message3 = array_shift($messages);
+        $mua3 = array_shift($muas);
-        $message4 = array_shift($messages);
+        $mua4 = array_shift($muas);
 
-        $this->assertNotEmpty($message1->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua1->userid);
-        $this->assertEmpty($message1->timeusertodeleted);
+        $this->assertEquals($m1id, $mua1->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action);
 
-        $this->assertEmpty($message2->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua2->userid);
-        $this->assertNotEmpty($message2->timeusertodeleted);
+        $this->assertEquals($m2id, $mua2->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action);
 
-        $this->assertNotEmpty($message3->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua3->userid);
-        $this->assertEmpty($message3->timeusertodeleted);
+        $this->assertEquals($m3id, $mua3->messageid);
-
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua3->action);
-        $this->assertEmpty($message4->timeuserfromdeleted);
-        $this->assertNotEmpty($message4->timeusertodeleted);
 
+        $this->assertEquals($user1->id, $mua4->userid);
+        $this->assertEquals($m4id, $mua4->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua4->action);
     }
 
     /**
@@ -1602,4 +1605,320 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->assertContains('Message 2', $message1->text);
         $this->assertContains('Message 3', $message2->text);
     }
+
+    /**
+     * Test returning blocked users.
+     */
+    public function test_get_blocked_users() {
+        global $USER;
+
+        // Set this user as the admin.
+        $this->setAdminUser();
+
+        // Create a user to add to the admin's contact list.
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+
+        // Add users to the admin's contact list.
+        message_add_contact($user1->id);
+        message_add_contact($user2->id, 1);
+
+        $this->assertCount(1, \core_message\api::get_blocked_users($USER->id));
+
+        // Block other user.
+        message_block_contact($user1->id);
+        $this->assertCount(2, \core_message\api::get_blocked_users($USER->id));
+
+        // Test deleting users.
+        delete_user($user1);
+        $this->assertCount(1, \core_message\api::get_blocked_users($USER->id));
+    }
+
+    /**
+     * Test returning contacts with unread message count.
+     */
+    public function test_get_contacts_with_unread_message_count() {
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+        $user3 = self::getDataGenerator()->create_user();
+        $user4 = self::getDataGenerator()->create_user();
+
+        // Add the users to each of their contacts.
+        message_add_contact($user1->id, 0, $user2->id);
+        message_add_contact($user2->id, 0, $user1->id);
+        message_add_contact($user3->id, 0, $user2->id);
+
+        $this->send_fake_message($user1, $user2);
+        $this->send_fake_message($user1, $user2);
+        $this->send_fake_message($user1, $user2);
+        $message4id = $this->send_fake_message($user1, $user2);
+
+        $this->send_fake_message($user3, $user2);
+        $message6id = $this->send_fake_message($user3, $user2);
+        $this->send_fake_message($user3, $user2);
+        $this->send_fake_message($user3, $user2);
+        $this->send_fake_message($user3, $user2);
+
+        // Send a message that should never be included as the user is not a contact.
+        $this->send_fake_message($user4, $user2);
+
+        // Get the contacts and the unread message count.
+        $messages = \core_message\api::get_contacts_with_unread_message_count($user2->id);
+
+        // Confirm the size is correct.
+        $this->assertCount(2, $messages);
+        ksort($messages);
+
+        $messageinfo1 = array_shift($messages);
+        $messageinfo2 = array_shift($messages);
+        $this->assertEquals($user1->id, $messageinfo1->id);
+        $this->assertEquals(4, $messageinfo1->messagecount);
+        $this->assertEquals($user3->id, $messageinfo2->id);
+        $this->assertEquals(5, $messageinfo2->messagecount);
+
+        // Mark some of the messages as read.
+        \core_message\api::mark_message_as_read($user2->id, $message4id);
+        \core_message\api::mark_message_as_read($user2->id, $message6id);
+
+        // Get the contacts and the unread message count.
+        $messages = \core_message\api::get_contacts_with_unread_message_count($user2->id);
+
+        // Confirm the size is correct.
+        $this->assertCount(2, $messages);
+        ksort($messages);
+
+        // Confirm read messages are not included.
+        $messageinfo1 = array_shift($messages);
+        $messageinfo2 = array_shift($messages);
+        $this->assertEquals($user1->id, $messageinfo1->id);
+        $this->assertEquals(3, $messageinfo1->messagecount);
+        $this->assertEquals($user3->id, $messageinfo2->id);
+        $this->assertEquals(4, $messageinfo2->messagecount);
+
+        // Now, let's populate the database with messages from user2 to user 1.
+        $this->send_fake_message($user2, $user1);
+        $this->send_fake_message($user2, $user1);
+        $messageid = $this->send_fake_message($user2, $user1);
+
+        // Send a message that should never be included as the user is not a contact.
+        $this->send_fake_message($user4, $user1);
+
+        // Get the contacts and the unread message count.
+        $messages = \core_message\api::get_contacts_with_unread_message_count($user1->id);
+
+        // Confirm the size is correct.
+        $this->assertCount(1, $messages);
+        $messageinfo1 = array_shift($messages);
+        $this->assertEquals($user2->id, $messageinfo1->id);
+        $this->assertEquals(3, $messageinfo1->messagecount);
+
+        // Mark the last message as read.
+        \core_message\api::mark_message_as_read($user1->id, $messageid);
+
+        $messages = \core_message\api::get_contacts_with_unread_message_count($user1->id);
+
+        // Confirm the size is correct.
+        $this->assertCount(1, $messages);
+
+        // Confirm read messages are not included.
+        $messageinfo1 = array_shift($messages);
+        $this->assertEquals($user2->id, $messageinfo1->id);
+        $this->assertEquals(2, $messageinfo1->messagecount);
+    }
+
+    /**
+     * Test returning contacts with unread message count when there are no messages.
+     */
+    public function test_get_contacts_with_unread_message_count_no_messages() {
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        // Add the users to each of their contacts.
+        message_add_contact($user1->id, 0, $user2->id);
+
+        // Check we get the correct message count.
+        $messages = \core_message\api::get_contacts_with_unread_message_count($user2->id);
+
+        // Confirm the size is correct.
+        $this->assertCount(1, $messages);
+
+        $messageinfo = array_shift($messages);
+
+        $this->assertEquals($user1->id, $messageinfo->id);
+        $this->assertEquals(0, $messageinfo->messagecount);
+    }
+
+    /**
+     * Test returning non-contacts with unread message count.
+     */
+    public function test_get_non_contacts_with_unread_message_count() {
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+        $user3 = self::getDataGenerator()->create_user();
+        $user4 = self::getDataGenerator()->create_user();
+
+        // Add a user to the contact list of the users we are testing this function with.
+        message_add_contact($user4->id, 0, $user1->id);
+        message_add_contact($user4->id, 0, $user2->id);
+
+        $this->send_fake_message($user1, $user2);
+        $this->send_fake_message($user1, $user2);
+        $this->send_fake_message($user1, $user2);
+        $message4id = $this->send_fake_message($user1, $user2);
+
+        $this->send_fake_message($user3, $user2);
+        $message6id = $this->send_fake_message($user3, $user2);
+        $this->send_fake_message($user3, $user2);
+        $this->send_fake_message($user3, $user2);
+        $this->send_fake_message($user3, $user2);
+
+        // Send a message that should never be included as the user is a contact.
+        $this->send_fake_message($user4, $user2);
+
+        // Get the non-contacts and the unread message count.
+        $messages = \core_message\api::get_non_contacts_with_unread_message_count($user2->id);
+
+        // Check we get the correct message count.
+        ksort($messages);
+        $this->assertCount(2, $messages);
+        $messageinfo1 = array_shift($messages);
+        $messageinfo2 = array_shift($messages);
+        $this->assertEquals($user1->id, $messageinfo1->id);
+        $this->assertEquals(4, $messageinfo1->messagecount);
+        $this->assertEquals($user3->id, $messageinfo2->id);
+        $this->assertEquals(5, $messageinfo2->messagecount);
+
+        // Mark some of the messages as read.
+        \core_message\api::mark_message_as_read($user2->id, $message4id);
+        \core_message\api::mark_message_as_read($user2->id, $message6id);
+
+        // Get the non-contacts and the unread message count.
+        $messages = \core_message\api::get_non_contacts_with_unread_message_count($user2->id);
+
+        // Check the marked message is not returned in the message count.
+        ksort($messages);
+        $this->assertCount(2, $messages);
+        $messageinfo1 = array_shift($messages);
+        $messageinfo2 = array_shift($messages);
+        $this->assertEquals($user1->id, $messageinfo1->id);
+        $this->assertEquals(3, $messageinfo1->messagecount);
+        $this->assertEquals($user3->id, $messageinfo2->id);
+        $this->assertEquals(4, $messageinfo2->messagecount);
+
+        // Now, let's populate the database with messages from user2 to user 1.
+        $this->send_fake_message($user2, $user1);
+        $this->send_fake_message($user2, $user1);
+        $messageid = $this->send_fake_message($user2, $user1);
+
+        // Send a message that should never be included as the user is a contact.
+        $this->send_fake_message($user4, $user1);
+
+        // Get the non-contacts and the unread message count.
+        $messages = \core_message\api::get_non_contacts_with_unread_message_count($user1->id);
+
+        // Confirm the size is correct.
+        $this->assertCount(1, $messages);
+        $messageinfo1 = array_shift($messages);
+        $this->assertEquals($user2->id, $messageinfo1->id);
+        $this->assertEquals(3, $messageinfo1->messagecount);
+
+        // Mark the last message as read.
+        \core_message\api::mark_message_as_read($user1->id, $messageid);
+
+        // Get the non-contacts and the unread message count.
+        $messages = \core_message\api::get_non_contacts_with_unread_message_count($user1->id);
+
+        // Check the marked message is not returned in the message count.
+        $this->assertCount(1, $messages);
+        $messageinfo1 = array_shift($messages);
+        $this->assertEquals($user2->id, $messageinfo1->id);
+        $this->assertEquals(2, $messageinfo1->messagecount);
+    }
+
+    /**
+     * Test marking a message as read.
+     */
+    public function test_mark_message_as_read() {
+        global $DB;
+
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        $this->send_fake_message($user1, $user2);
+        $m2id = $this->send_fake_message($user1, $user2);
+        $this->send_fake_message($user2, $user1);
+        $m4id = $this->send_fake_message($user2, $user1);
+
+        \core_message\api::mark_message_as_read($user2->id, $m2id, 11);
+        \core_message\api::mark_message_as_read($user1->id, $m4id, 12);
+
+        // Confirm there are two user actions.
+        $muas = $DB->get_records('message_user_actions', [], 'timecreated ASC');
+        $this->assertEquals(2, count($muas));
+
+        // Confirm they are correct.
+        $mua1 = array_shift($muas);
+        $mua2 = array_shift($muas);
+
+        // Confirm first action.
+        $this->assertEquals($user2->id, $mua1->userid);
+        $this->assertEquals($m2id, $mua1->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_READ, $mua1->action);
+        $this->assertEquals(11, $mua1->timecreated);
+
+        // Confirm second action.
+        $this->assertEquals($user1->id, $mua2->userid);
+        $this->assertEquals($m4id, $mua2->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_READ, $mua2->action);
+        $this->assertEquals(12, $mua2->timecreated);
+    }
+
+    /**
+     * Test marking a notification as read.
+     */
+    public function test_mark_notification_as_read() {
+        global $DB;
+
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        $this->send_fake_message($user1, $user2, 'Notification 1', 1);
+        $n2id = $this->send_fake_message($user1, $user2, 'Notification 2', 1);
+        $this->send_fake_message($user2, $user1, 'Notification 3', 1);
+        $n4id = $this->send_fake_message($user2, $user1, 'Notification 4', 1);
+
+        \core_message\api::mark_notification_as_read($user2->id, $n2id, 11);
+        \core_message\api::mark_notification_as_read($user1->id, $n4id, 12);
+
+        // Retrieve the notifications.
+        $n2 = $DB->get_record('notifications', ['id' => $n2id]);
+        $n4 = $DB->get_record('notifications', ['id' => $n4id]);
+
+        // Confirm they have been marked as read.
+        $this->assertEquals(11, $n2->timeread);
+        $this->assertEquals(12, $n4->timeread);
+    }
+
+    /**
+     * Test a conversation is not returned if there is none.
+     */
+    public function test_get_conversation_between_users_no_conversation() {
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        $this->assertFalse(\core_message\api::get_conversation_between_users($user1->id, $user2->id));
+    }
+
+    /**
+     * Test we can return a conversation that exists between users.
+     */
+    public function test_get_conversation_between_users_with_existing_conversation() {
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        $conversationid = \core_message\api::create_conversation_between_users($user1->id, $user2->id);
+
+        $this->assertEquals($conversationid,
+            \core_message\api::get_conversation_between_users($user1->id, $user2->id));
+    }
 }

message/tests/events_test.php

@@ -25,7 +25,19 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-class core_message_events_testcase extends advanced_testcase {
+global $CFG;
+
+require_once($CFG->dirroot . '/message/tests/messagelib_test.php');
+
+/**
+ * Class containing the tests for message related events.
+ *
+ * @package core_message
+ * @category test
+ * @copyright 2014 Mark Nelson <markn@moodle.com>
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class core_message_events_testcase extends core_message_messagelib_testcase {
 
     /**
      * Test set up.
@@ -197,11 +209,11 @@ class core_message_events_testcase extends advanced_testcase {
      */
     public function test_message_sent() {
         $event = \core\event\message_sent::create(array(
+            'objectid' => 3,
             'userid' => 1,
             'context'  => context_system::instance(),
             'relateduserid' => 2,
             'other' => array(
-                'messageid' => 3,
                 'courseid' => 4
             )
         ));
@@ -219,7 +231,7 @@ class core_message_events_testcase extends advanced_testcase {
         $this->assertEventLegacyLogData($expected, $event);
         $url = new moodle_url('/message/index.php', array('user1' => $event->userid, 'user2' => $event->relateduserid));
         $this->assertEquals($url, $event->get_url());
-        $this->assertEquals(3, $event->other['messageid']);
+        $this->assertEquals(3, $event->objectid);
         $this->assertEquals(4, $event->other['courseid']);
     }
 
@@ -256,7 +268,7 @@ class core_message_events_testcase extends advanced_testcase {
         $this->assertEventLegacyLogData($expected, $event);
         $url = new moodle_url('/message/index.php', array('user1' => $event->userid, 'user2' => $event->relateduserid));
         $this->assertEquals($url, $event->get_url());
-        $this->assertEquals(3, $event->other['messageid']);
+        $this->assertEquals(3, $event->objectid);
         $this->assertEquals(4, $event->other['courseid']);
     }
 
@@ -276,32 +288,33 @@ class core_message_events_testcase extends advanced_testcase {
         $this->assertEquals(SITEID, $event->other['courseid']);
     }
 
-
-
-
     /**
      * Test the message viewed event.
      */
     public function test_message_viewed() {
         global $DB;
 
-        // Create a message to mark as read.
+        // Create users to send messages between.
-        $message = new stdClass();
+        $user1 = $this->getDataGenerator()->create_user();
-        $message->useridfrom = '1';
+        $user2 = $this->getDataGenerator()->create_user();
-        $message->useridto = '2';
+
-        $message->subject = 'Subject';
+        $messageid = $this->send_fake_message($user1, $user2);
-        $message->message = 'Message';
-        $message->id = $DB->insert_record('message', $message);
 
         // Trigger and capture the event.
         $sink = $this->redirectEvents();
-        message_mark_message_read($message, time());
+        \core_message\api::mark_message_as_read($user1->id, $messageid);
         $events = $sink->get_events();
         $event = reset($events);
 
+        // Get the usage action.
+        $mua = $DB->get_record('message_user_actions', ['userid' => $user1->id, 'messageid' => $messageid,
+            'action' => \core_message\api::MESSAGE_ACTION_READ]);
+
         // Check that the event data is valid.
         $this->assertInstanceOf('\core\event\message_viewed', $event);
-        $this->assertEquals(context_user::instance(2), $event->get_context());
+        $this->assertEquals(context_user::instance($user1->id), $event->get_context());
+        $this->assertEquals($mua->id, $event->objectid);
+        $this->assertEquals($messageid, $event->other['messageid']);
         $url = new moodle_url('/message/index.php', array('user1' => $event->userid, 'user2' => $event->relateduserid));
         $this->assertEquals($url, $event->get_url());
     }
@@ -312,56 +325,53 @@ class core_message_events_testcase extends advanced_testcase {
     public function test_message_deleted() {
         global $DB;
 
-        // Create a message.
+        // Create users to send messages between.
-        $message = new stdClass();
+        $user1 = $this->getDataGenerator()->create_user();
-        $message->useridfrom = '1';
+        $user2 = $this->getDataGenerator()->create_user();
-        $message->useridto = '2';
+
-        $message->subject = 'Subject';
+        $messageid = $this->send_fake_message($user1, $user2);
-        $message->message = 'Message';
-        $message->timeuserfromdeleted = 0;
-        $message->timeusertodeleted = 0;
-        $message->id = $DB->insert_record('message', $message);
 
         // Trigger and capture the event.
         $sink = $this->redirectEvents();
-        message_delete_message($message, $message->useridfrom);
+        \core_message\api::delete_message($user1->id, $messageid);
         $events = $sink->get_events();
         $event = reset($events);
 
+        // Get the usage action.
+        $mua = $DB->get_record('message_user_actions', ['userid' => $user1->id, 'messageid' => $messageid,
+            'action' => \core_message\api::MESSAGE_ACTION_DELETED]);
+
         // Check that the event data is valid.
         $this->assertInstanceOf('\core\event\message_deleted', $event);
-        $this->assertEquals($message->useridfrom, $event->userid); // The user who deleted it.
+        $this->assertEquals($user1->id, $event->userid); // The user who deleted it.
-        $this->assertEquals($message->useridto, $event->relateduserid);
+        $this->assertEquals($user2->id, $event->relateduserid);
-        $this->assertEquals('message', $event->other['messagetable']);
+        $this->assertEquals($mua->id, $event->objectid);
-        $this->assertEquals($message->id, $event->other['messageid']);
+        $this->assertEquals($messageid, $event->other['messageid']);
-        $this->assertEquals($message->useridfrom, $event->other['useridfrom']);
+        $this->assertEquals($user1->id, $event->other['useridfrom']);
-        $this->assertEquals($message->useridto, $event->other['useridto']);
+        $this->assertEquals($user2->id, $event->other['useridto']);
 
         // Create a read message.
-        $message = new stdClass();
+        $messageid = $this->send_fake_message($user1, $user2);
-        $message->useridfrom = '2';
+        \core_message\api::mark_message_as_read($user2->id, $messageid);
-        $message->useridto = '1';
-        $message->subject = 'Subject';
-        $message->message = 'Message';
-        $message->timeuserfromdeleted = 0;
-        $message->timeusertodeleted = 0;
-        $message->timeread = time();
-        $message->id = $DB->insert_record('message_read', $message);
 
         // Trigger and capture the event.
         $sink = $this->redirectEvents();
-        message_delete_message($message, $message->useridto);
+        \core_message\api::delete_message($user2->id, $messageid);
         $events = $sink->get_events();
         $event = reset($events);
 
+        // Get the usage action.
+        $mua = $DB->get_record('message_user_actions', ['userid' => $user2->id, 'messageid' => $messageid,
+            'action' => \core_message\api::MESSAGE_ACTION_DELETED]);
+
         // Check that the event data is valid.
         $this->assertInstanceOf('\core\event\message_deleted', $event);
-        $this->assertEquals($message->useridto, $event->userid);
+        $this->assertEquals($user2->id, $event->userid);
-        $this->assertEquals($message->useridfrom, $event->relateduserid);
+        $this->assertEquals($user1->id, $event->relateduserid);
-        $this->assertEquals('message_read', $event->other['messagetable']);
+        $this->assertEquals($mua->id, $event->objectid);
-        $this->assertEquals($message->id, $event->other['messageid']);
+        $this->assertEquals($messageid, $event->other['messageid']);
-        $this->assertEquals($message->useridfrom, $event->other['useridfrom']);
+        $this->assertEquals($user1->id, $event->other['useridfrom']);
-        $this->assertEquals($message->useridto, $event->other['useridto']);
+        $this->assertEquals($user2->id, $event->other['useridto']);
     }
 
     /**
@@ -370,65 +380,65 @@ class core_message_events_testcase extends advanced_testcase {
     public function test_message_deleted_whole_conversation() {
         global $DB;
 
-        // Create a message.
+        // Create some users.
-        $message = new stdClass();
+        $user1 = self::getDataGenerator()->create_user();
-        $message->useridfrom = '1';
+        $user2 = self::getDataGenerator()->create_user();
-        $message->useridto = '2';
-        $message->subject = 'Subject';
-        $message->message = 'Message';
-        $message->timeuserfromdeleted = 0;
-        $message->timeusertodeleted = 0;
-        $message->timecreated = 1;
 
+        // The person doing the search.
+        $this->setUser($user1);
+
+        // Send some messages back and forth.
+        $time = 1;
         $messages = [];
-        // Send this a few times.
+        $messages[] = $this->send_fake_message($user1, $user2, 'Yo!', 0, $time + 1);
-        $messages[] = $DB->insert_record('message', $message);
+        $messages[] = $this->send_fake_message($user2, $user1, 'Sup mang?', 0, $time + 2);
+        $messages[] = $this->send_fake_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 3);
+        $messages[] = $this->send_fake_message($user2, $user1, 'Word.', 0, $time + 4);
+        $messages[] = $this->send_fake_message($user1, $user2, 'You doing much?', 0, $time + 5);
+        $messages[] = $this->send_fake_message($user2, $user1, 'Nah', 0, $time + 6);
+        $messages[] = $this->send_fake_message($user1, $user2, 'You nubz0r!', 0, $time + 7);
+        $messages[] = $this->send_fake_message($user2, $user1, 'Ouch.', 0, $time + 8);
 
-        $message->timecreated++;
+        // Mark the last 4 messages as read.
-        $messages[] = $DB->insert_record('message', $message);
+        \core_message\api::mark_message_as_read($user2->id, $messages[4]);
-
+        \core_message\api::mark_message_as_read($user1->id, $messages[5]);
-        $message->timecreated++;
+        \core_message\api::mark_message_as_read($user2->id, $messages[6]);
-        $messages[] = $DB->insert_record('message', $message);
+        \core_message\api::mark_message_as_read($user1->id, $messages[7]);
-
-        $message->timecreated++;
-        $messages[] = $DB->insert_record('message', $message);
-
-        // Create a read message.
-        $message->timeread = time();
-
-        // Send this a few times.
-        $message->timecreated++;
-        $messages[] = $DB->insert_record('message_read', $message);
-
-        $message->timecreated++;
-        $messages[] = $DB->insert_record('message_read', $message);
-
-        $message->timecreated++;
-        $messages[] = $DB->insert_record('message_read', $message);
-
-        $message->timecreated++;
-        $messages[] = $DB->insert_record('message_read', $message);
 
         // Trigger and capture the event.
         $sink = $this->redirectEvents();
-        \core_message\api::delete_conversation(1, 2);
+        \core_message\api::delete_conversation($user1->id, $user2->id);
         $events = $sink->get_events();
 
+        // Get the user actions for the messages deleted by that user.
+        $muas = $DB->get_records('message_user_actions', ['userid' => $user1->id,
+            'action' => \core_message\api::MESSAGE_ACTION_DELETED], 'timecreated ASC');
+        $this->assertCount(8, $muas);
+
+        // Create a list we can use for testing.
+        $muatest = [];
+        foreach ($muas as $mua) {
+            $muatest[$mua->messageid] = $mua;
+        }
+
         // Check that there were the correct number of events triggered.
         $this->assertEquals(8, count($events));
 
         // Check that the event data is valid.
-        $i = 0;
+        $i = 1;
         foreach ($events as $event) {
-            $table = ($i > 3) ? 'message_read' : 'message';
+            $useridfromid = ($i % 2 == 0) ? $user2->id : $user1->id;
+            $useridtoid = ($i % 2 == 0) ? $user1->id : $user2->id;
+            $messageid = $messages[$i - 1];
 
             $this->assertInstanceOf('\core\event\message_deleted', $event);
-            $this->assertEquals($message->useridfrom, $event->userid);
+
-            $this->assertEquals($message->useridto, $event->relateduserid);
+            $this->assertEquals($muatest[$messageid]->id, $event->objectid);
-            $this->assertEquals($table, $event->other['messagetable']);
+            $this->assertEquals($user1->id, $event->userid);
-            $this->assertEquals($messages[$i], $event->other['messageid']);
+            $this->assertEquals($user2->id, $event->relateduserid);
-            $this->assertEquals($message->useridfrom, $event->other['useridfrom']);
+            $this->assertEquals($messageid, $event->other['messageid']);
-            $this->assertEquals($message->useridto, $event->other['useridto']);
+            $this->assertEquals($useridfromid, $event->other['useridfrom']);
+            $this->assertEquals($useridtoid, $event->other['useridto']);
 
             $i++;
         }

message/tests/externallib_test.php

@@ -61,16 +61,33 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
             $time = time();
         }
 
+        if ($notification) {
+            $record = new stdClass();
+            $record->useridfrom = $userfrom->id;
+            $record->useridto = $userto->id;
+            $record->subject = 'No subject';
+            $record->fullmessage = $message;
+            $record->smallmessage = $message;
+            $record->timecreated = $time;
+
+            return $DB->insert_record('notifications', $record);
+        }
+
+        if (!$conversationid = \core_message\api::get_conversation_between_users($userfrom->id, $userto->id)) {
+            $conversationid = \core_message\api::create_conversation_between_users($userfrom->id,
+                $userto->id);
+        }
+
+        // Ok, send the message.
         $record = new stdClass();
         $record->useridfrom = $userfrom->id;
-        $record->useridto = $userto->id;
+        $record->conversationid = $conversationid;
         $record->subject = 'No subject';
-        $record->smallmessage = $message;
         $record->fullmessage = $message;
+        $record->smallmessage = $message;
         $record->timecreated = $time;
-        $record->notification = $notification;
 
-        return $DB->insert_record('message', $record);
+        return $DB->insert_record('messages', $record);
     }
 
     /**
@@ -110,7 +127,15 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         // We need to execute the return values cleaning process to simulate the web service server.
         $sentmessages = external_api::clean_returnvalue(core_message_external::send_instant_messages_returns(), $sentmessages);
 
-        $themessage = $DB->get_record('message', array('id' => $sentmessages[0]['msgid']));
+        $sql = "SELECT m.*, mcm.userid as useridto
+                 FROM {messages} m
+           INNER JOIN {message_conversations} mc
+                   ON m.conversationid = mc.id
+           INNER JOIN {message_conversation_members} mcm
+                   ON mcm.conversationid = mc.id
+                WHERE mcm.userid != ?
+                  AND m.id = ?";
+        $themessage = $DB->get_record_sql($sql, [$USER->id, $sentmessages[0]['msgid']]);
 
         // Confirm that the message was inserted correctly.
         $this->assertEquals($themessage->useridfrom, $USER->id);
@@ -465,8 +490,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         // Delete the message.
         $message = array_shift($messages['messages']);
-        $messagetobedeleted = $DB->get_record('message_read', array('id' => $message['id']));
+        \core_message\api::delete_message($user1->id, $message['id']);
-        message_delete_message($messagetobedeleted, $user1->id);
 
         $messages = core_message_external::get_messages($user2->id, $user1->id, 'conversations', true, true, 0, 0);
         $messages = external_api::clean_returnvalue(core_message_external::get_messages_returns(), $messages);
@@ -495,8 +519,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         // Delete the message.
         $message = array_shift($messages['messages']);
-        $messagetobedeleted = $DB->get_record('message_read', array('id' => $message['id']));
+        \core_message\api::delete_message($user2->id, $message['id']);
-        message_delete_message($messagetobedeleted, $user2->id);
 
         $messages = core_message_external::get_messages($user2->id, $user3->id, 'conversations', true, true, 0, 0);
         $messages = external_api::clean_returnvalue(core_message_external::get_messages_returns(), $messages);
@@ -735,10 +758,10 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         // Invalid message ids.
         try {
-            $messageid = core_message_external::mark_message_read($messageids[0]['messageid'] * 2, time());
+            $messageid = core_message_external::mark_message_read(1337, time());
             $this->fail('Exception expected due invalid messageid.');
         } catch (dml_missing_record_exception $e) {
-            $this->assertEquals('invalidrecord', $e->errorcode);
+            $this->assertEquals('invalidrecordunknown', $e->errorcode);
         }
 
         // A message to a different user.
@@ -783,8 +806,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $result = external_api::clean_returnvalue(core_message_external::delete_message_returns(), $result);
         $this->assertTrue($result['status']);
         $this->assertCount(0, $result['warnings']);
-        $deletedmessage = $DB->get_record('message', array('id' => $m1to2));
+        $mua = $DB->get_record('message_user_actions', array('messageid' => $m1to2, 'userid' => $user1->id));
-        $this->assertNotEquals(0, $deletedmessage->timeuserfromdeleted);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua->action);
 
         // Try to delete the same message again.
         $result = core_message_external::delete_message($m1to2, $user1->id, false);
@@ -805,25 +828,24 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $result = external_api::clean_returnvalue(core_message_external::delete_message_returns(), $result);
         $this->assertTrue($result['status']);
         $this->assertCount(0, $result['warnings']);
-        $deletedmessage = $DB->get_record('message', array('id' => $m2to3));
+        $this->assertTrue($DB->record_exists('message_user_actions', array('messageid' => $m2to3, 'userid' => $user3->id,
-        $this->assertNotEquals(0, $deletedmessage->timeusertodeleted);
+            'action' => \core_message\api::MESSAGE_ACTION_DELETED)));
 
         // Delete a message read.
-        $message = $DB->get_record('message', array('id' => $m3to2));
+        \core_message\api::mark_message_as_read($user3->id, $m3to2, time());
-        $messageid = message_mark_message_read($message, time());
+        $result = core_message_external::delete_message($m3to2, $user3->id);
-        $result = core_message_external::delete_message($messageid, $user3->id);
         $result = external_api::clean_returnvalue(core_message_external::delete_message_returns(), $result);
         $this->assertTrue($result['status']);
         $this->assertCount(0, $result['warnings']);
-        $deletedmessage = $DB->get_record('message_read', array('id' => $messageid));
+        $this->assertTrue($DB->record_exists('message_user_actions', array('messageid' => $m3to2, 'userid' => $user3->id,
-        $this->assertNotEquals(0, $deletedmessage->timeuserfromdeleted);
+            'action' => \core_message\api::MESSAGE_ACTION_DELETED)));
 
         // Invalid message ids.
         try {
             $result = core_message_external::delete_message(-1, $user1->id);
             $this->fail('Exception expected due invalid messageid.');
         } catch (dml_missing_record_exception $e) {
-            $this->assertEquals('invalidrecord', $e->errorcode);
+            $this->assertEquals('invalidrecordunknown', $e->errorcode);
         }
 
         // Invalid user.
@@ -849,8 +871,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $result = external_api::clean_returnvalue(core_message_external::delete_message_returns(), $result);
         $this->assertTrue($result['status']);
         $this->assertCount(0, $result['warnings']);
-        $deletedmessage = $DB->get_record('message', array('id' => $m3to4));
+        $this->assertTrue($DB->record_exists('message_user_actions', array('messageid' => $m3to4, 'userid' => $user4->id,
-        $this->assertNotEquals(0, $deletedmessage->timeusertodeleted);
+            'action' => \core_message\api::MESSAGE_ACTION_DELETED)));
 
     }
 
@@ -902,15 +924,15 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $this->send_message($sender3, $recipient, 'Notification', 1);
 
         core_message_external::mark_all_notifications_as_read($recipient->id, $sender1->id);
-        $readnotifications = $DB->get_records('message_read', ['useridto' => $recipient->id]);
+        $readnotifications = $DB->get_records_select('notifications', 'useridto = ? AND timeread IS NOT NULL', [$recipient->id]);
-        $unreadnotifications = $DB->get_records('message', ['useridto' => $recipient->id]);
+        $unreadnotifications = $DB->get_records_select('notifications', 'useridto = ? AND timeread IS NULL', [$recipient->id]);
 
         $this->assertCount(2, $readnotifications);
         $this->assertCount(4, $unreadnotifications);
 
         core_message_external::mark_all_notifications_as_read($recipient->id, 0);
-        $readnotifications = $DB->get_records('message_read', ['useridto' => $recipient->id]);
+        $readnotifications = $DB->get_records_select('notifications', 'useridto = ? AND timeread IS NOT NULL', [$recipient->id]);
-        $unreadnotifications = $DB->get_records('message', ['useridto' => $recipient->id]);
+        $unreadnotifications = $DB->get_records_select('notifications', 'useridto = ? AND timeread IS NULL', [$recipient->id]);
 
         $this->assertCount(6, $readnotifications);
         $this->assertCount(0, $unreadnotifications);
@@ -2400,18 +2422,10 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $this->send_message($sender3, $recipient, 'Message');
 
         core_message_external::mark_all_messages_as_read($recipient->id, $sender1->id);
-        $readnotifications = $DB->get_records('message_read', array('useridto' => $recipient->id));
+        $this->assertEquals(2, $DB->count_records('message_user_actions'));
-        $unreadnotifications = $DB->get_records('message', array('useridto' => $recipient->id));
-
-        $this->assertCount(2, $readnotifications);
-        $this->assertCount(4, $unreadnotifications);
 
         core_message_external::mark_all_messages_as_read($recipient->id, 0);
-        $readnotifications = $DB->get_records('message_read', array('useridto' => $recipient->id));
+        $this->assertEquals(6, $DB->count_records('message_user_actions'));
-        $unreadnotifications = $DB->get_records('message', array('useridto' => $recipient->id));
-
-        $this->assertCount(6, $readnotifications);
-        $this->assertCount(0, $unreadnotifications);
     }
 
     /**
@@ -2529,33 +2543,39 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         // Send some messages back and forth.
         $time = time();
-        $this->send_message($user1, $user2, 'Yo!', 0, $time);
+        $m1id = $this->send_message($user1, $user2, 'Yo!', 0, $time);
-        $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1);
+        $m2id = $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1);
-        $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2);
+        $m3id = $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2);
-        $this->send_message($user2, $user1, 'Word.', 0, $time + 3);
+        $m4id = $this->send_message($user2, $user1, 'Word.', 0, $time + 3);
 
         // Delete the conversation.
         core_message_external::delete_conversation($user1->id, $user2->id);
 
-        $messages = $DB->get_records('message', array(), 'timecreated ASC');
+        $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC');
-        $this->assertCount(4, $messages);
+        $this->assertCount(4, $muas);
+        // Sort by id.
+        ksort($muas);
 
-        $message1 = array_shift($messages);
+        $mua1 = array_shift($muas);
-        $message2 = array_shift($messages);
+        $mua2 = array_shift($muas);
-        $message3 = array_shift($messages);
+        $mua3 = array_shift($muas);
-        $message4 = array_shift($messages);
+        $mua4 = array_shift($muas);
 
-        $this->assertNotEmpty($message1->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua1->userid);
-        $this->assertEmpty($message1->timeusertodeleted);
+        $this->assertEquals($m1id, $mua1->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action);
 
-        $this->assertEmpty($message2->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua2->userid);
-        $this->assertNotEmpty($message2->timeusertodeleted);
+        $this->assertEquals($m2id, $mua2->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action);
 
-        $this->assertNotEmpty($message3->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua3->userid);
-        $this->assertEmpty($message3->timeusertodeleted);
+        $this->assertEquals($m3id, $mua3->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua3->action);
 
-        $this->assertEmpty($message4->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua4->userid);
-        $this->assertNotEmpty($message4->timeusertodeleted);
+        $this->assertEquals($m4id, $mua4->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua4->action);
     }
 
     /**
@@ -2574,33 +2594,39 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         // Send some messages back and forth.
         $time = time();
-        $this->send_message($user1, $user2, 'Yo!', 0, $time);
+        $m1id = $this->send_message($user1, $user2, 'Yo!', 0, $time);
-        $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1);
+        $m2id = $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1);
-        $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2);
+        $m3id = $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2);
-        $this->send_message($user2, $user1, 'Word.', 0, $time + 3);
+        $m4id = $this->send_message($user2, $user1, 'Word.', 0, $time + 3);
 
         // Delete the conversation.
         core_message_external::delete_conversation($user1->id, $user2->id);
 
-        $messages = $DB->get_records('message', array(), 'timecreated ASC');
+        $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC');
-        $this->assertCount(4, $messages);
+        $this->assertCount(4, $muas);
+        // Sort by id.
+        ksort($muas);
 
-        $message1 = array_shift($messages);
+        $mua1 = array_shift($muas);
-        $message2 = array_shift($messages);
+        $mua2 = array_shift($muas);
-        $message3 = array_shift($messages);
+        $mua3 = array_shift($muas);
-        $message4 = array_shift($messages);
+        $mua4 = array_shift($muas);
 
-        $this->assertNotEmpty($message1->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua1->userid);
-        $this->assertEmpty($message1->timeusertodeleted);
+        $this->assertEquals($m1id, $mua1->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action);
 
-        $this->assertEmpty($message2->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua2->userid);
-        $this->assertNotEmpty($message2->timeusertodeleted);
+        $this->assertEquals($m2id, $mua2->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action);
 
-        $this->assertNotEmpty($message3->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua3->userid);
-        $this->assertEmpty($message3->timeusertodeleted);
+        $this->assertEquals($m3id, $mua3->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua3->action);
 
-        $this->assertEmpty($message4->timeuserfromdeleted);
+        $this->assertEquals($user1->id, $mua4->userid);
-        $this->assertNotEmpty($message4->timeusertodeleted);
+        $this->assertEquals($m4id, $mua4->messageid);
+        $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua4->action);
     }
 
     /**

message/tests/messagelib_test.php

@@ -73,16 +73,33 @@ class core_message_messagelib_testcase extends advanced_testcase {
             $time = time();
         }
 
+        if ($notification) {
+            $record = new stdClass();
+            $record->useridfrom = $userfrom->id;
+            $record->useridto = $userto->id;
+            $record->subject = 'No subject';
+            $record->fullmessage = $message;
+            $record->smallmessage = $message;
+            $record->timecreated = $time;
+
+            return $DB->insert_record('notifications', $record);
+        }
+
+        if (!$conversationid = \core_message\api::get_conversation_between_users($userfrom->id, $userto->id)) {
+            $conversationid = \core_message\api::create_conversation_between_users($userfrom->id,
+                $userto->id);
+        }
+
+        // Ok, send the message.
         $record = new stdClass();
         $record->useridfrom = $userfrom->id;
-        $record->useridto = $userto->id;
+        $record->conversationid = $conversationid;
         $record->subject = 'No subject';
         $record->fullmessage = $message;
         $record->smallmessage = $message;
         $record->timecreated = $time;
-        $record->notification = $notification;
 
-        return $DB->insert_record('message', $record);
+        return $DB->insert_record('messages', $record);
     }
 
     /**
@@ -101,14 +118,17 @@ class core_message_messagelib_testcase extends advanced_testcase {
         message_add_contact($user2->id, 1);
 
         $this->assertCount(1, message_get_blocked_users());
+        $this->assertDebuggingCalled();
 
         // Block other user.
         message_block_contact($user1->id);
         $this->assertCount(2, message_get_blocked_users());
+        $this->assertDebuggingCalled();
 
         // Test deleting users.
         delete_user($user1);
         $this->assertCount(1, message_get_blocked_users());
+        $this->assertDebuggingCalled();
     }
 
     /**
@@ -138,6 +158,7 @@ class core_message_messagelib_testcase extends advanced_testcase {
         $this->send_fake_message($user3, $USER);
 
         list($onlinecontacts, $offlinecontacts, $strangers) = message_get_contacts();
+        $this->assertDebuggingCalled();
         $this->assertCount(0, $onlinecontacts);
         $this->assertCount(2, $offlinecontacts);
         $this->assertCount(1, $strangers);
@@ -146,6 +167,7 @@ class core_message_messagelib_testcase extends advanced_testcase {
         $this->send_fake_message($noreplyuser, $USER);
         $this->send_fake_message($supportuser, $USER);
         list($onlinecontacts, $offlinecontacts, $strangers) = message_get_contacts();
+        $this->assertDebuggingCalled();
         $this->assertCount(0, $onlinecontacts);
         $this->assertCount(2, $offlinecontacts);
         $this->assertCount(3, $strangers);
@@ -153,6 +175,7 @@ class core_message_messagelib_testcase extends advanced_testcase {
         // Block 1 user.
         message_block_contact($user2->id);
         list($onlinecontacts, $offlinecontacts, $strangers) = message_get_contacts();
+        $this->assertDebuggingCalled();
         $this->assertCount(0, $onlinecontacts);
         $this->assertCount(1, $offlinecontacts);
         $this->assertCount(3, $strangers);
@@ -161,6 +184,7 @@ class core_message_messagelib_testcase extends advanced_testcase {
         core_user::reset_internal_users();
         $CFG->noreplyuserid = $user3->id;
         list($onlinecontacts, $offlinecontacts, $strangers) = message_get_contacts();
+        $this->assertDebuggingCalled();
         $this->assertCount(0, $onlinecontacts);
         $this->assertCount(1, $offlinecontacts);
         $this->assertCount(2, $strangers);
@@ -168,8 +192,9 @@ class core_message_messagelib_testcase extends advanced_testcase {
         // Test deleting users.
         delete_user($user1);
         delete_user($user3);
-
+        core_user::reset_internal_users();
         list($onlinecontacts, $offlinecontacts, $strangers) = message_get_contacts();
+        $this->assertDebuggingCalled();
         $this->assertCount(0, $onlinecontacts);
         $this->assertCount(0, $offlinecontacts);
         $this->assertCount(1, $strangers);
@@ -195,9 +220,9 @@ class core_message_messagelib_testcase extends advanced_testcase {
     }
 
     /**
-     * Test message_count_unread_messages with notifications.
+     * Test message_count_unread_messages with read messages.
      */
-    public function test_message_count_unread_messages_with_notifications() {
+    public function test_message_count_unread_messages_with_read_messages() {
         // Create users to send and receive messages.
         $userfrom1 = $this->getDataGenerator()->create_user();
         $userfrom2 = $this->getDataGenerator()->create_user();
@@ -206,16 +231,15 @@ class core_message_messagelib_testcase extends advanced_testcase {
         $this->assertEquals(0, message_count_unread_messages($userto));
 
         // Send fake messages.
-        $this->send_fake_message($userfrom1, $userto);
+        $messageid = $this->send_fake_message($userfrom1, $userto);
         $this->send_fake_message($userfrom2, $userto);
 
-        // Send fake notifications.
+        // Mark message as read.
-        $this->send_fake_message($userfrom1, $userto, 'Notification', 1);
+        \core_message\api::mark_message_as_read($userto->id, $messageid);
-        $this->send_fake_message($userfrom2, $userto, 'Notification', 1);
 
-        // Should only count the messages.
+        // Should only count the messages that weren't read by the current user.
-        $this->assertEquals(2, message_count_unread_messages($userto));
+        $this->assertEquals(1, message_count_unread_messages($userto));
-        $this->assertEquals(1, message_count_unread_messages($userto, $userfrom1));
+        $this->assertEquals(0, message_count_unread_messages($userto, $userfrom1));
     }
 
     /**
@@ -235,13 +259,8 @@ class core_message_messagelib_testcase extends advanced_testcase {
         $messageid = $this->send_fake_message($userfrom1, $userto);
         $this->send_fake_message($userfrom2, $userto);
 
-        // Send fake notifications.
-        $this->send_fake_message($userfrom1, $userto, 'Notification', 1);
-        $this->send_fake_message($userfrom2, $userto, 'Notification', 1);
-
         // Delete a message.
-        $message = $DB->get_record('message', array('id' => $messageid));
+        \core_message\api::delete_message($userto->id, $messageid);
-        message_delete_message($message, $userto->id);
 
         // Should only count the messages that weren't deleted by the current user.
         $this->assertEquals(1, message_count_unread_messages($userto));

message/tests/search_received_test.php

@@ -273,7 +273,7 @@ class message_received_search_testcase extends advanced_testcase {
             $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($messageid));
         }
 
-        message_delete_message($message, $user2->id);
+        \core_message\api::delete_message($user2->id, $message->id);
         $this->assertEquals(\core_search\manager::ACCESS_DELETED, $searcharea->check_access($messageid));
 
         $this->setUser($user3);

message/tests/search_sent_test.php

@@ -284,7 +284,7 @@ class message_sent_search_testcase extends advanced_testcase {
 
         $this->assertEquals(\core_search\manager::ACCESS_DELETED, $searcharea->check_access(-123));
 
-        message_delete_message($message, $user1->id);
+        \core_message\api::delete_message($user1->id, $message->id);
         $this->assertEquals(\core_search\manager::ACCESS_DELETED, $searcharea->check_access($messageid));
 
         $this->setUser($user2);