diff --git a/message/lib.php b/message/lib.php index e3de0aba86a..a3bb7cb69d3 100644 --- a/message/lib.php +++ b/message/lib.php @@ -704,56 +704,70 @@ function message_print_search($advancedsearch = false, $user1=null) { function message_get_recent_conversations($user, $limitfrom=0, $limitto=100) { global $DB; - $userfields = user_picture::fields('otheruser', array('lastaccess')); + $userfields = user_picture::fields('u', array('lastaccess')); + //This query retrieves the last message received from and sent to each user + //It unions that data then, within that set, it finds the most recent message you've exchanged with each user over all + //It then joins with some other tables to get some additional data we need - // This query retrieves the most recent message received from or sent to - // seach other user. - // - // If two messages have the same timecreated, we take the one with the - // larger id. - // - // There is a separate query for read and unread messages as they are stored - // in different tables. They were originally retrieved in one query but it - // was so large that it was difficult to be confident in its correctness. - $sql = "SELECT $userfields, - message.id as mid, message.notification, message.smallmessage, message.fullmessage, - message.fullmessagehtml, message.fullmessageformat, message.timecreated, - contact.id as contactlistid, contact.blocked + //message ID is used instead of timecreated as it should sort the same and will be much faster - FROM {message_read} message - JOIN {user} otheruser ON otheruser.id = CASE - WHEN message.useridto = :userid1 THEN message.useridfrom - ELSE message.useridto END - LEFT JOIN {message_contacts} contact ON contact.userid = :userid2 AND contact.contactid = otheruser.id + //There is a separate query for read and unread queries as they are stored in different tables + //They were originally retrieved in one query but it was so large that it was difficult to be confident in its correctness + $sql = "SELECT $userfields, mr.id as mid, mr.notification, mr.smallmessage, mr.fullmessage, mr.fullmessagehtml, mr.fullmessageformat, mr.timecreated, mc.id as contactlistid, mc.blocked + FROM {message_read} mr + JOIN ( + SELECT messages.userid AS userid, MAX(messages.mid) AS mid + FROM ( + SELECT mr1.useridto AS userid, MAX(mr1.id) AS mid + FROM {message_read} mr1 + WHERE mr1.useridfrom = :userid1 + AND mr1.notification = 0 + GROUP BY mr1.useridto + UNION + SELECT mr2.useridfrom AS userid, MAX(mr2.id) AS mid + FROM {message_read} mr2 + WHERE mr2.useridto = :userid2 + AND mr2.notification = 0 + GROUP BY mr2.useridfrom + ) messages + GROUP BY messages.userid + ) messages2 ON mr.id = messages2.mid AND (mr.useridto = messages2.userid OR mr.useridfrom = messages2.userid) + JOIN {user} u ON u.id = messages2.userid + LEFT JOIN {message_contacts} mc ON mc.userid = :userid3 AND mc.contactid = u.id + WHERE u.deleted = '0' + ORDER BY mr.id DESC"; + $params = array('userid1' => $user->id, 'userid2' => $user->id, 'userid3' => $user->id); + $read = $DB->get_records_sql($sql, $params, $limitfrom, $limitto); - WHERE otheruser.deleted = 0 - AND (message.useridto = :userid3 OR message.useridfrom = :userid4) - AND message.notification = 0 - AND NOT EXISTS ( - SELECT 1 - FROM {message_read} othermessage - WHERE ((othermessage.useridto = :userid5 AND othermessage.useridfrom = otheruser.id) OR - (othermessage.useridfrom = :userid6 AND othermessage.useridto = otheruser.id)) - AND (othermessage.timecreated > message.timecreated OR ( - othermessage.timecreated = message.timecreated AND othermessage.id > message.id)) - ) - - ORDER BY message.timecreated DESC"; - $params = array('userid1' => $user->id, 'userid2' => $user->id, 'userid3' => $user->id, - 'userid4' => $user->id, 'userid5' => $user->id, 'userid6' => $user->id); - $read = $DB->get_records_sql($sql, $params, $limitfrom, $limitto); - - // We want to get the messages that have not been read. These are stored in the 'message' table. It is the - // exact same query as the one above, except for the table we are querying. So, simply replace references to - // the 'message_read' table with the 'message' table. - $sql = str_replace('{message_read}', '{message}', $sql); - $unread = $DB->get_records_sql($sql, $params, $limitfrom, $limitto); + $sql = "SELECT $userfields, m.id as mid, m.notification, m.smallmessage, m.fullmessage, m.fullmessagehtml, m.fullmessageformat, m.timecreated, mc.id as contactlistid, mc.blocked + FROM {message} m + JOIN ( + SELECT messages.userid AS userid, MAX(messages.mid) AS mid + FROM ( + SELECT m1.useridto AS userid, MAX(m1.id) AS mid + FROM {message} m1 + WHERE m1.useridfrom = :userid1 + AND m1.notification = 0 + GROUP BY m1.useridto + UNION + SELECT m2.useridfrom AS userid, MAX(m2.id) AS mid + FROM {message} m2 + WHERE m2.useridto = :userid2 + AND m2.notification = 0 + GROUP BY m2.useridfrom + ) messages + GROUP BY messages.userid + ) messages2 ON m.id = messages2.mid AND (m.useridto = messages2.userid OR m.useridfrom = messages2.userid) + JOIN {user} u ON u.id = messages2.userid + LEFT JOIN {message_contacts} mc ON mc.userid = :userid3 AND mc.contactid = u.id + WHERE u.deleted = '0' + ORDER BY m.id DESC"; + $unread = $DB->get_records_sql($sql, $params, $limitfrom, $limitto); $conversations = array(); - // Union the 2 result sets together looking for the message with the most - // recent timecreated for each other user. - // $conversation->id (the array key) is the other user's ID. + //Union the 2 result sets together looking for the message with the most recent timecreated for each other user + //$conversation->id (the array key) is the other user's ID $conversation_arrays = array($unread, $read); foreach ($conversation_arrays as $conversation_array) { foreach ($conversation_array as $conversation) { @@ -788,7 +802,7 @@ function message_get_recent_notifications($user, $limitfrom=0, $limitto=100) { FROM {message_read} mr JOIN {user} u ON u.id=mr.useridfrom WHERE mr.useridto = :userid1 AND u.deleted = '0' AND mr.notification = :notification - ORDER BY mr.timecreated DESC"; + ORDER BY mr.id DESC";//ordering by id should give the same result as ordering by timecreated but will be faster $params = array('userid1' => $user->id, 'notification' => 1); $notifications = $DB->get_records_sql($sql, $params, $limitfrom, $limitto); diff --git a/message/tests/messagelib_test.php b/message/tests/messagelib_test.php index 0259d5781e0..126e6afd02a 100644 --- a/message/tests/messagelib_test.php +++ b/message/tests/messagelib_test.php @@ -62,7 +62,6 @@ class core_message_messagelib_testcase extends advanced_testcase { * @param stdClass $userfrom user object of the one sending the message. * @param stdClass $userto user object of the one receiving the message. * @param string $message message to send. - * @return int the id of the message */ protected function send_fake_message($userfrom, $userto, $message = 'Hello world!') { global $DB; @@ -73,8 +72,7 @@ class core_message_messagelib_testcase extends advanced_testcase { $record->subject = 'No subject'; $record->fullmessage = $message; $record->timecreated = time(); - - return $DB->insert_record('message', $record); + $insert = $DB->insert_record('message', $record); } /** @@ -360,81 +358,4 @@ class core_message_messagelib_testcase extends advanced_testcase { $this->assertEquals(false, message_search(array('Message'), true, true, 2)); $this->assertCount(5, message_search(array('Message'), true, true, SITEID)); } - - /** - * Test message_get_recent_conversations. - */ - public function test_message_get_recent_conversations() { - global $DB, $USER; - - // Set this user as the admin. - $this->setAdminUser(); - - // Create user's to send messages to/from. - $user1 = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'user1')); - $user2 = $this->getDataGenerator()->create_user(array('firstname' => 'Test2', 'lastname' => 'user2')); - - // Add a few messages that have been read and some that are unread. - $m1 = $this->send_fake_message($USER, $user1, 'Message 1'); // An unread message. - $m2 = $this->send_fake_message($user1, $USER, 'Message 2'); // An unread message. - $m3 = $this->send_fake_message($USER, $user1, 'Message 3'); // An unread message. - $m4 = message_post_message($USER, $user2, 'Message 4', FORMAT_PLAIN); - $m5 = message_post_message($user2, $USER, 'Message 5', FORMAT_PLAIN); - $m6 = message_post_message($USER, $user2, 'Message 6', FORMAT_PLAIN); - - // We want to alter the timecreated values so we can ensure message_get_recent_conversations orders by timecreated. - $updatemessage = new stdClass(); - $updatemessage->id = $m3; - $updatemessage->timecreated = 0; - $DB->update_record('message', $updatemessage); - - $updatemessage->id = $m6; - $DB->update_record('message_read', $updatemessage); - - // Get the recent conversations for the current user. - $conversations = message_get_recent_conversations($USER); - - // Confirm that we have received the messages with the maximum timecreated, rather than the max id. - $this->assertEquals('Message 2', $conversations[0]->fullmessage); - $this->assertEquals('Message 5', $conversations[1]->smallmessage); - } - - /** - * Test message_get_recent_notifications. - */ - public function test_message_get_recent_notifications() { - global $DB, $USER; - - // Set this user as the admin. - $this->setAdminUser(); - - // Create a user to send messages from. - $user1 = $this->getDataGenerator()->create_user(array('firstname' => 'Test1', 'lastname' => 'user1')); - - // Add two messages - will mark them as notifications later. - $m1 = message_post_message($user1, $USER, 'Message 1', FORMAT_PLAIN); - $m2 = message_post_message($user1, $USER, 'Message 2', FORMAT_PLAIN); - - // Mark the second message as a notification. - $updatemessage = new stdClass(); - $updatemessage->id = $m2; - $updatemessage->notification = 1; - $DB->update_record('message_read', $updatemessage); - - // Mark the first message as a notification and change the timecreated to 0. - $updatemessage->id = $m1; - $updatemessage->notification = 1; - $updatemessage->timecreated = 0; - $DB->update_record('message_read', $updatemessage); - - $notifications = message_get_recent_notifications($USER); - - // Get the messages. - $firstmessage = array_shift($notifications); - $secondmessage = array_shift($notifications); - - // Confirm that we have received the notifications with the maximum timecreated, rather than the max id. - $this->assertEquals('Message 2', $firstmessage->smallmessage); - $this->assertEquals('Message 1', $secondmessage->smallmessage); - } }