diff --git a/lib/classes/message/manager.php b/lib/classes/message/manager.php index d08489db601..9047748da3d 100644 --- a/lib/classes/message/manager.php +++ b/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( - $eventdata->userfrom->id, - $eventdata->userto->id, - $savemessage->id, - $eventdata->courseid + if (!$eventdata->notification) { + \core\event\message_sent::create_from_ids( + $eventdata->userfrom->id, + $eventdata->userto->id, + $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( - $eventdata->userfrom->id, - $eventdata->userto->id, - $savemessage->id, - $eventdata->courseid + if (!$eventdata->notification) { + \core\event\message_sent::create_from_ids( + $eventdata->userfrom->id, + $eventdata->userto->id, + $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; } /** diff --git a/lib/classes/task/messaging_cleanup_task.php b/lib/classes/task/messaging_cleanup_task.php index 4debce36c0d..6d1a2c4d69f 100644 --- a/lib/classes/task/messaging_cleanup_task.php +++ b/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); } } diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index 132eb55d84e..12031708afe 100644 --- a/lib/deprecatedlib.php +++ b/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); +} diff --git a/lib/messagelib.php b/lib/messagelib.php index d575cbc82aa..04b25c62ac9 100644 --- a/lib/messagelib.php +++ b/lib/messagelib.php @@ -127,33 +127,53 @@ function message_send($eventdata) { $userstate = 'loggedoff'; } - // Create the message object - $savemessage = new stdClass(); - $savemessage->courseid = $eventdata->courseid; - $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; + // Check if we are creating a notification or message. + if ($eventdata->notification) { + $table = 'notifications'; - if (!empty($eventdata->contexturl)) { - $savemessage->contexturl = (string)$eventdata->contexturl; + $tabledata = new stdClass(); + $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)) { - $savemessage->contexturlname = (string)$eventdata->contexturlname; - } else { - $savemessage->contexturlname = null; - } - - $savemessage->timecreated = time(); + $tabledata->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('message_read', $savemessage); - $message = $DB->get_record('message_read', array('id'=>$messageid)); + $messageid = $DB->insert_record($table, $tabledata); + $message = $DB->get_record($table, 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, - $savemessage->useridto); - $cache->set($key, $savemessage->timecreated); + $key = \core_message\helper::get_last_message_time_created_cache_key($eventdata->userfrom->id, + $eventdata->userto->id); + $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); - $eventdata->savedmessageid = $savemessage->id; + $tabledata->id = $DB->insert_record($table, $tabledata); + $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); } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index a8259ae5eab..978720afe3e 100644 --- a/lib/moodlelib.php +++ b/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. diff --git a/lib/phpunit/classes/message_sink.php b/lib/phpunit/classes/message_sink.php index ffbf83e75d6..383780c170f 100644 --- a/lib/phpunit/classes/message_sink.php +++ b/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. * diff --git a/lib/phpunit/classes/phpmailer_sink.php b/lib/phpunit/classes/phpmailer_sink.php index bb00478144e..df0eb015afd 100644 --- a/lib/phpunit/classes/phpmailer_sink.php +++ b/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. * diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index d5cfd9462d2..3252bf6854d 100644 --- a/lib/phpunit/classes/util.php +++ b/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) { diff --git a/lib/tests/message_test.php b/lib/tests/message_test.php index 42c4cad72e5..3f1d3deb983 100644 --- a/lib/tests/message_test.php +++ b/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); diff --git a/lib/tests/messagelib_test.php b/lib/tests/messagelib_test.php index 051f61c0880..1dd36481c0f 100644 --- a/lib/tests/messagelib_test.php +++ b/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())); - $DB->delete_records('message_read', array()); + $this->assertTrue($DB->record_exists('message_user_actions', array('userid' => $user2->id, 'messageid' => $messageid, + '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 = '

message body

'; $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())); - $DB->delete_records('message_read', array()); + $this->assertTrue($DB->record_exists('message_user_actions', array('userid' => $user2->id, 'messageid' => $messageid, + '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('message_read', array())); + $this->assertFalse($DB->record_exists('messages', 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())); - $DB->delete_records('message', array()); + $this->assertFalse($DB->record_exists('message_user_actions', 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())); - $DB->delete_records('message_read', array()); + $this->assertTrue($DB->record_exists('message_user_actions', array('userid' => $user2->id, 'messageid' => $messageid, + '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())); - $DB->delete_records('message_read', array()); + $this->assertFalse($DB->record_exists('messages', array())); + $DB->delete_records('notifications', array()); $events = $eventsink->get_events(); - $this->assertCount(2, $events); - $this->assertInstanceOf('\core\event\message_sent', $events[0]); - $this->assertInstanceOf('\core\event\message_viewed', $events[1]); + $this->assertCount(0, $events); $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())); - $DB->delete_records('message', array()); + $this->assertFalse($DB->record_exists('message_user_actions', 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())); - $DB->delete_records('message_read', array()); + $this->assertFalse($DB->record_exists('message_user_actions', 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())); - $DB->delete_records('message', array()); + $this->assertFalse($DB->record_exists('message_user_actions', 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())); - $DB->delete_records('message', array()); + $this->assertFalse($DB->record_exists('message_user_actions', 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())); - $DB->delete_records('message', array()); + $this->assertFalse($DB->record_exists('message_user_actions', 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->assertFalse($DB->record_exists('message_read', array())); + $this->assertCount(3, $DB->get_records('messages')); + $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('message_read', array()); + $DB->delete_records('messages', array()); $transaction = $DB->start_delegated_transaction(); message_send($message); message_send($message); - $this->assertCount(2, $DB->get_records('message')); - $this->assertCount(0, $DB->get_records('message_read')); + $this->assertCount(2, $DB->get_records('messages')); + $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('message_read')); + $this->assertCount(0, $DB->get_records('messages')); message_send($message); - $this->assertCount(1, $DB->get_records('message')); - $this->assertCount(0, $DB->get_records('message_read')); + $this->assertCount(1, $DB->get_records('messages')); + $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() { diff --git a/message/classes/api.php b/message/classes/api.php index 20655292126..83719137e72 100644 --- a/message/classes/api.php +++ b/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, m.useridto, m.subject, m.fullmessage, m.fullmessagehtml, m.fullmessageformat, - m.smallmessage, m.notification, m.timecreated, 0 as isread, $ufields, mc.blocked as userfrom_blocked, - $ufields2, mc2.blocked as userto_blocked - FROM {message} m - JOIN {user} u - ON m.useridfrom = u.id - LEFT JOIN {message_contacts} mc - ON (mc.contactid = u.id AND mc.userid = ?) - JOIN {user} u2 - ON m.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 " . $DB->sql_like('smallmessage', '?', false) . " - UNION ALL - 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 + $sql = "SELECT m.id, m.useridfrom, mcm.userid as useridto, m.subject, m.fullmessage, m.fullmessagehtml, m.fullmessageformat, + m.smallmessage, m.timecreated, 0 as isread, $ufields, mcont.blocked as userfrom_blocked, $ufields2, + mcont2.blocked as userto_blocked + FROM {messages} m + INNER JOIN {user} u + ON u.id = m.useridfrom + INNER JOIN {message_conversations} mc + ON mc.id = m.conversationid + INNER JOIN {message_conversation_members} mcm + ON mcm.conversationid = m.conversationid + INNER JOIN {user} u2 + ON u2.id = mcm.userid + LEFT JOIN {message_contacts} mcont + ON (mcont.contactid = u.id AND mcont.userid = ?) + LEFT JOIN {message_contacts} mcont2 + ON (mcont2.contactid = u2.id AND mcont2.userid = ?) + LEFT JOIN {message_user_actions} mua + ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ?) + WHERE (m.useridfrom = ? OR mcm.userid = ?) + AND m.useridfrom != mcm.userid 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 - // whether a user sent or received a message (it's the same conversation). - // E.g. If there is a message from user 1 to user 2 and then from user 2 to user 1 the result set - // will group those into a single record, since 1 -> 2 and 2 -> 1 is the same conversation. - $case1 = $DB->sql_concat('useridfrom', "'-'", 'useridto'); - $case2 = $DB->sql_concat('useridto', "'-'", 'useridfrom'); - $convocase = "CASE WHEN useridfrom > useridto - THEN $case1 - ELSE $case2 END"; - $convosig = "$convocase AS convo_signature"; + // Get the last message from each conversation that the user belongs to. + $sql = "SELECT m.id, m.conversationid, m.useridfrom, mcm2.userid as useridto, m.smallmessage, m.timecreated + FROM {messages} m + INNER JOIN ( + SELECT MAX(m.id) AS messageid + FROM {messages} m + INNER JOIN ( + SELECT m.conversationid, MAX(m.timecreated) as maxtime + FROM {messages} m + 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; - if (!isset($messages[$id]) || empty($message->timeread)) { - $messages[$id] = $message; - } + $messages[$message->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} - WHERE id $useridsql - AND deleted = 0"; + FROM {user} u + WHERE id $useridsql + 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 - // specifically been separated into another query to avoid having to do joins on the message - // tables. - $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); + // If there are no other users (user may have been deleted), then do not continue. + if (empty($otherusers)) { + return []; } + $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 - // 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)); + $conversationid = self::get_conversation_between_users($userid, $otheruserid); - $sql = "UPDATE {message} - SET timeusertodeleted = :time - WHERE useridto = :userid - AND useridfrom = :otheruserid - AND notification = 0"; - $DB->execute($sql, array('time' => $now, 'userid' => $userid, 'otheruserid' => $otheruserid)); + // If there is no conversation, there is nothing to do. + if (!$conversationid) { + return true; + } - $sql = "UPDATE {message_read} - SET timeuserfromdeleted = :time - WHERE useridfrom = :userid - AND useridto = :otheruserid - AND notification = 0"; - $DB->execute($sql, array('time' => $now, 'userid' => $userid, 'otheruserid' => $otheruserid)); + // Get all messages belonging to this conversation that have not already been deleted by this user. + $sql = "SELECT m.* + FROM {messages} m + INNER JOIN {message_conversations} mc + ON m.conversationid = mc.id + 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} - SET timeusertodeleted = :time - WHERE useridto = :userid - AND useridfrom = :otheruserid - AND notification = 0"; - $DB->execute($sql, array('time' => $now, 'userid' => $userid, 'otheruserid' => $otheruserid)); + // Ok, mark these as deleted. + foreach ($messages as $message) { + $mua = new \stdClass(); + $mua->userid = $userid; + $mua->messageid = $message->id; + $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 ($messages = helper::get_messages($userid, $otheruserid, $now)) { - // Loop through and trigger a deleted event. - foreach ($messages as $message) { - $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(); + if ($message->useridfrom == $userid) { + $useridto = $otheruserid; + } else { + $useridto = $userid; } + \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( - 'message', - 'useridto = ? AND timeusertodeleted = 0 AND notification = 0', - [$user->id], - "COUNT(DISTINCT(useridfrom))"); + $sql = "SELECT COUNT(DISTINCT(m.conversationid)) + FROM {messages} m + INNER JOIN {message_conversations} mc + ON m.conversationid = mc.id + 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(); - - if (!empty($touserid)) { - $params['useridto'] = $touserid; - } + $type = strtolower($type); + // 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; + } } diff --git a/message/classes/helper.php b/message/classes/helper.php index b7f2a041e31..c780bd7afc0 100644 --- a/message/classes/helper.php +++ b/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'); - $messagereadid = $DB->sql_concat("'messageread_'", 'id'); + $sql = "SELECT m.id, m.useridfrom, mdm.userid as useridto, m.subject, m.fullmessage, m.fullmessagehtml, + 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, - smallmessage, notification, timecreated, 0 as timeread - FROM {message} m - WHERE ((useridto = ? AND useridfrom = ? AND timeusertodeleted = ?) - OR (useridto = ? AND useridfrom = ? AND timeuserfromdeleted = ?)) - AND notification = 0 - %where% - UNION ALL - SELECT {$messagereadid} AS fakeid, id, useridfrom, useridto, subject, fullmessage, fullmessagehtml, fullmessageformat, - smallmessage, notification, timecreated, timeread - FROM {message_read} mr - 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); + if (empty($timedeleted)) { + $sql .= " LEFT JOIN {message_user_actions} mua + ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ? AND mua.timecreated is NOT NULL)"; + $params[] = $userid; + $params[] = api::MESSAGE_ACTION_DELETED; + } else { + $sql .= " INNER JOIN {message_user_actions} mua + ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ? AND mua.timecreated = ?)"; + $params[] = $userid; + $params[] = api::MESSAGE_ACTION_DELETED; + $params[] = $timedeleted; + } - $params2 = array($userid, $otheruserid, $timedeleted, - $otheruserid, $userid, $timedeleted); - $where = array(); + $sql .= " WHERE (m.useridfrom = ? AND mdm.userid = ? OR m.useridfrom = ? AND mdm.userid = ?)"; + $params = array_merge($params, [$userid, $otheruserid, $otheruserid, $userid]); if (!empty($timefrom)) { - $where[] = 'AND timecreated >= ?'; - $params1[] = $timefrom; - $params2[] = $timefrom; + $sql .= " AND m.timecreated >= ?"; + $params[] = $timefrom; } if (!empty($timeto)) { - $where[] = 'AND timecreated <= ?'; - $params1[] = $timeto; - $params2[] = $timeto; + $sql .= " AND m.timecreated <= ?"; + $params[] = $timeto; } - $sql = str_replace('%where%', implode(' ', $where), $sql); - $params = array_merge($params1, $params2); + if (empty($timedeleted)) { + $sql .= " AND mua.id is NULL"; + } + + $sql .= " ORDER BY m.$sort"; return $DB->get_records_sql($sql, $params, $limitfrom, $limitnum); } diff --git a/message/classes/search/base_message.php b/message/classes/search/base_message.php index 35fce7fc34f..58479f25423 100644 --- a/message/classes/search/base_message.php +++ b/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); } } diff --git a/message/classes/search/message_received.php b/message/classes/search/message_received.php index 26a2561c137..052d6bde907 100644 --- a/message/classes/search/message_received.php +++ b/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; } diff --git a/message/classes/search/message_sent.php b/message/classes/search/message_sent.php index 9425a82b19a..79dc0bdb4f1 100644 --- a/message/classes/search/message_sent.php +++ b/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; } diff --git a/message/externallib.php b/message/externallib.php index 173b1c420f0..d8b0ce4c3e0 100644 --- a/message/externallib.php +++ b/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' => $online, 'offline' => $offline, 'strangers' => $strangers); - foreach ($allcontacts as $mode => $contacts) { - foreach ($contacts as $key => $contact) { - $newcontact = array( - 'id' => $contact->id, - 'fullname' => fullname($contact), - 'unread' => $contact->messagecount - ); + $allcontacts = array('online' => [], 'offline' => [], 'strangers' => []); + $contacts = \core_message\api::get_contacts_with_unread_message_count($USER->id); + foreach ($contacts as $contact) { + // Set the mode. + $mode = 'offline'; + if (\core_message\helper::is_online($contact->lastaccess)) { + $mode = 'online'; + } - $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); + $newcontact = array( + 'id' => $contact->id, + 'fullname' => fullname($contact), + 'unread' => $contact->messagecount + ); - $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]); - continue; + unset($messages[$mid]); + 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 (message_can_delete_message($message, $user->id)) { - $status = message_delete_message($message, $user->id);; + if (\core_message\api::can_delete_message($user->id, $messageid)) { + $status = \core_message\api::delete_message($user->id, $messageid); } else { throw new moodle_exception('You do not have permission to delete this message'); } diff --git a/message/lib.php b/message/lib.php index 90f7a293ef4..9143d130a52 100644 --- a/message/lib.php +++ b/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 - AND timeusertodeleted = 0", - 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')"); + $sql .= " AND m.useridfrom = ?"; + $params[] = $user2->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}'; - $params = array('deleted' => 0); - - // Empty useridto means that we are going to retrieve messages send by the useridfrom to any user. + // If the 'useridto' value is empty then we are going to retrieve messages sent 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"; - $usersql = "mr.useridto = :useridto AND (u.deleted IS NULL OR u.deleted = :deleted)"; - $params['useridto'] = $useridto; + $messagesql .= "LEFT JOIN {user} u + ON u.id = mr.useridfrom + $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"; - $params['useridfrom'] = $useridfrom; + $messagesql .= " AND mr.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. - $typesql = ""; - if ($notifications !== -1) { - $typesql = "AND mr.notification = :notification"; - $params['notification'] = ($notifications) ? 1 : 0; + // Handle messages if needed. + if ($notifications === -1 || $notifications === 0) { + $messages = $DB->get_records_sql($messagesql, $messageparams, $limitfrom, $limitnum); + // Get rid of the messages that have either been read or not read depending on the value of $read. + $messages = array_filter($messages, function ($message) use ($read) { + if ($read) { + return !is_null($message->timeread); + } + + return is_null($message->timeread); + }); } - $sql = "SELECT mr.*, $userfields - FROM $messagetable mr - $joinsql - WHERE $usersql - $typesql - ORDER BY $sort"; + // All. + if ($notifications === -1) { + return array_merge($messages, $DB->get_records_sql($notificationsql, $notificationsparams, $limitfrom, $limitnum)); + } else if ($notifications === 1) { // Just notifications. + return $DB->get_records_sql($notificationsql, $notificationsparams, $limitfrom, $limitnum); + } - $messages = $DB->get_records_sql($sql, $params, $limitfrom, $limitnum); + // Just messages. return $messages; } diff --git a/message/output/popup/classes/api.php b/message/output/popup/classes/api.php index 84826f61981..5592f40420d 100644 --- a/message/output/popup/classes/api.php +++ b/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, - r.subject, r.fullmessage, r.fullmessageformat, - r.fullmessagehtml, r.smallmessage, r.notification, r.contexturl, - r.contexturlname, r.timecreated, r.timeuserfromdeleted, r.timeusertodeleted, - r.component, r.eventtype, r.timeread - FROM {message_read} r - WHERE r.notification = 1 - AND r.id IN (SELECT messageid FROM {message_popup} WHERE isread = 1) - 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 + $sql = "SELECT n.id, n.useridfrom, n.useridto, + n.subject, n.fullmessage, n.fullmessageformat, + n.fullmessagehtml, n.smallmessage, n.contexturl, + n.contexturlname, n.timecreated, n.component, + n.eventtype, n.timeread + FROM {notifications} n + WHERE n.id IN (SELECT messageid FROM {message_popup}) + AND n.useridto = :useridto1 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} - WHERE id IN (SELECT messageid FROM {message_popup} WHERE isread = 0) - AND useridto = ?", + FROM {notifications} + WHERE id IN (SELECT messageid FROM {message_popup} WHERE isread = 0) + AND useridto = ?", [$useridto] ); } diff --git a/message/output/popup/classes/output/popup_notification.php b/message/output/popup/classes/output/popup_notification.php index 0644a223967..0511873237a 100644 --- a/message/output/popup/classes/output/popup_notification.php +++ b/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; diff --git a/message/output/popup/externallib.php b/message/output/popup/externallib.php index 87e93b0795b..ffbc092f246 100644 --- a/message/output/popup/externallib.php +++ b/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; } } diff --git a/message/output/popup/message_output_popup.php b/message/output/popup/message_output_popup.php index 67409cc43b9..84ecee34e24 100644 --- a/message/output/popup/message_output_popup.php +++ b/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); diff --git a/message/output/popup/tests/base.php b/message/output/popup/tests/base.php index 2d082ae2cce..52bb8d568b1 100644 --- a/message/output/popup/tests/base.php +++ b/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; diff --git a/message/output/popup/tests/behat/message_popover_unread.feature b/message/output/popup/tests/behat/message_popover_unread.feature index fdee7956493..67289a28f81 100644 --- a/message/output/popup/tests/behat/message_popover_unread.feature +++ b/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 diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 7139800c460..7030a7444ae 100644 --- a/message/tests/api_test.php +++ b/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); - $this->send_fake_message($user2, $user1, 'Sup mang?', 0, $time + 2); - $this->send_fake_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 3); - $this->send_fake_message($user2, $user1, 'Word.', 0, $time + 4); + $m1id = $this->send_fake_message($user1, $user2, 'Yo!', 0, $time + 1); + $m2id = $this->send_fake_message($user2, $user1, 'Sup mang?', 0, $time + 2); + $m3id = $this->send_fake_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 3); + $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'); - $this->assertCount(4, $messages); + $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC'); + $this->assertCount(4, $muas); + // Sort by id. + ksort($muas); - $message1 = array_shift($messages); - $message2 = array_shift($messages); - $message3 = array_shift($messages); - $message4 = array_shift($messages); + $mua1 = array_shift($muas); + $mua2 = array_shift($muas); + $mua3 = array_shift($muas); + $mua4 = array_shift($muas); - $this->assertNotEmpty($message1->timeuserfromdeleted); - $this->assertEmpty($message1->timeusertodeleted); + $this->assertEquals($user1->id, $mua1->userid); + $this->assertEquals($m1id, $mua1->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action); - $this->assertEmpty($message2->timeuserfromdeleted); - $this->assertNotEmpty($message2->timeusertodeleted); + $this->assertEquals($user1->id, $mua2->userid); + $this->assertEquals($m2id, $mua2->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action); - $this->assertNotEmpty($message3->timeuserfromdeleted); - $this->assertEmpty($message3->timeusertodeleted); - - $this->assertEmpty($message4->timeuserfromdeleted); - $this->assertNotEmpty($message4->timeusertodeleted); + $this->assertEquals($user1->id, $mua3->userid); + $this->assertEquals($m3id, $mua3->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua3->action); + $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)); + } } diff --git a/message/tests/events_test.php b/message/tests/events_test.php index 4006738c234..e05bf83d286 100644 --- a/message/tests/events_test.php +++ b/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 + * @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. - $message = new stdClass(); - $message->useridfrom = '1'; - $message->useridto = '2'; - $message->subject = 'Subject'; - $message->message = 'Message'; - $message->id = $DB->insert_record('message', $message); + // Create users to send messages between. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + + $messageid = $this->send_fake_message($user1, $user2); // 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. - $message = new stdClass(); - $message->useridfrom = '1'; - $message->useridto = '2'; - $message->subject = 'Subject'; - $message->message = 'Message'; - $message->timeuserfromdeleted = 0; - $message->timeusertodeleted = 0; - $message->id = $DB->insert_record('message', $message); + // Create users to send messages between. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + + $messageid = $this->send_fake_message($user1, $user2); // 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($message->useridto, $event->relateduserid); - $this->assertEquals('message', $event->other['messagetable']); - $this->assertEquals($message->id, $event->other['messageid']); - $this->assertEquals($message->useridfrom, $event->other['useridfrom']); - $this->assertEquals($message->useridto, $event->other['useridto']); + $this->assertEquals($user1->id, $event->userid); // The user who deleted it. + $this->assertEquals($user2->id, $event->relateduserid); + $this->assertEquals($mua->id, $event->objectid); + $this->assertEquals($messageid, $event->other['messageid']); + $this->assertEquals($user1->id, $event->other['useridfrom']); + $this->assertEquals($user2->id, $event->other['useridto']); // Create a read message. - $message = new stdClass(); - $message->useridfrom = '2'; - $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); + $messageid = $this->send_fake_message($user1, $user2); + \core_message\api::mark_message_as_read($user2->id, $messageid); // 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($message->useridfrom, $event->relateduserid); - $this->assertEquals('message_read', $event->other['messagetable']); - $this->assertEquals($message->id, $event->other['messageid']); - $this->assertEquals($message->useridfrom, $event->other['useridfrom']); - $this->assertEquals($message->useridto, $event->other['useridto']); + $this->assertEquals($user2->id, $event->userid); + $this->assertEquals($user1->id, $event->relateduserid); + $this->assertEquals($mua->id, $event->objectid); + $this->assertEquals($messageid, $event->other['messageid']); + $this->assertEquals($user1->id, $event->other['useridfrom']); + $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. - $message = new stdClass(); - $message->useridfrom = '1'; - $message->useridto = '2'; - $message->subject = 'Subject'; - $message->message = 'Message'; - $message->timeuserfromdeleted = 0; - $message->timeusertodeleted = 0; - $message->timecreated = 1; + // Create some users. + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + // The person doing the search. + $this->setUser($user1); + + // Send some messages back and forth. + $time = 1; $messages = []; - // Send this a few times. - $messages[] = $DB->insert_record('message', $message); + $messages[] = $this->send_fake_message($user1, $user2, 'Yo!', 0, $time + 1); + $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++; - $messages[] = $DB->insert_record('message', $message); - - $message->timecreated++; - $messages[] = $DB->insert_record('message', $message); - - $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); + // Mark the last 4 messages as read. + \core_message\api::mark_message_as_read($user2->id, $messages[4]); + \core_message\api::mark_message_as_read($user1->id, $messages[5]); + \core_message\api::mark_message_as_read($user2->id, $messages[6]); + \core_message\api::mark_message_as_read($user1->id, $messages[7]); // 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($table, $event->other['messagetable']); - $this->assertEquals($messages[$i], $event->other['messageid']); - $this->assertEquals($message->useridfrom, $event->other['useridfrom']); - $this->assertEquals($message->useridto, $event->other['useridto']); + + $this->assertEquals($muatest[$messageid]->id, $event->objectid); + $this->assertEquals($user1->id, $event->userid); + $this->assertEquals($user2->id, $event->relateduserid); + $this->assertEquals($messageid, $event->other['messageid']); + $this->assertEquals($useridfromid, $event->other['useridfrom']); + $this->assertEquals($useridtoid, $event->other['useridto']); $i++; } diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index b7392f18833..047740c50d9 100644 --- a/message/tests/externallib_test.php +++ b/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'])); - message_delete_message($messagetobedeleted, $user1->id); + \core_message\api::delete_message($user1->id, $message['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'])); - message_delete_message($messagetobedeleted, $user2->id); + \core_message\api::delete_message($user2->id, $message['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)); - $this->assertNotEquals(0, $deletedmessage->timeuserfromdeleted); + $mua = $DB->get_record('message_user_actions', array('messageid' => $m1to2, 'userid' => $user1->id)); + $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->assertNotEquals(0, $deletedmessage->timeusertodeleted); + $this->assertTrue($DB->record_exists('message_user_actions', array('messageid' => $m2to3, 'userid' => $user3->id, + 'action' => \core_message\api::MESSAGE_ACTION_DELETED))); // Delete a message read. - $message = $DB->get_record('message', array('id' => $m3to2)); - $messageid = message_mark_message_read($message, time()); - $result = core_message_external::delete_message($messageid, $user3->id); + \core_message\api::mark_message_as_read($user3->id, $m3to2, time()); + $result = core_message_external::delete_message($m3to2, $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->assertNotEquals(0, $deletedmessage->timeuserfromdeleted); + $this->assertTrue($DB->record_exists('message_user_actions', array('messageid' => $m3to2, 'userid' => $user3->id, + '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->assertNotEquals(0, $deletedmessage->timeusertodeleted); + $this->assertTrue($DB->record_exists('message_user_actions', array('messageid' => $m3to4, 'userid' => $user4->id, + '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]); - $unreadnotifications = $DB->get_records('message', ['useridto' => $recipient->id]); + $readnotifications = $DB->get_records_select('notifications', 'useridto = ? AND timeread IS NOT NULL', [$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]); - $unreadnotifications = $DB->get_records('message', ['useridto' => $recipient->id]); + $readnotifications = $DB->get_records_select('notifications', 'useridto = ? AND timeread IS NOT NULL', [$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)); - $unreadnotifications = $DB->get_records('message', array('useridto' => $recipient->id)); - - $this->assertCount(2, $readnotifications); - $this->assertCount(4, $unreadnotifications); + $this->assertEquals(2, $DB->count_records('message_user_actions')); core_message_external::mark_all_messages_as_read($recipient->id, 0); - $readnotifications = $DB->get_records('message_read', array('useridto' => $recipient->id)); - $unreadnotifications = $DB->get_records('message', array('useridto' => $recipient->id)); - - $this->assertCount(6, $readnotifications); - $this->assertCount(0, $unreadnotifications); + $this->assertEquals(6, $DB->count_records('message_user_actions')); } /** @@ -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); - $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); - $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); - $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + $m1id = $this->send_message($user1, $user2, 'Yo!', 0, $time); + $m2id = $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $m3id = $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $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'); - $this->assertCount(4, $messages); + $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC'); + $this->assertCount(4, $muas); + // Sort by id. + ksort($muas); - $message1 = array_shift($messages); - $message2 = array_shift($messages); - $message3 = array_shift($messages); - $message4 = array_shift($messages); + $mua1 = array_shift($muas); + $mua2 = array_shift($muas); + $mua3 = array_shift($muas); + $mua4 = array_shift($muas); - $this->assertNotEmpty($message1->timeuserfromdeleted); - $this->assertEmpty($message1->timeusertodeleted); + $this->assertEquals($user1->id, $mua1->userid); + $this->assertEquals($m1id, $mua1->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action); - $this->assertEmpty($message2->timeuserfromdeleted); - $this->assertNotEmpty($message2->timeusertodeleted); + $this->assertEquals($user1->id, $mua2->userid); + $this->assertEquals($m2id, $mua2->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action); - $this->assertNotEmpty($message3->timeuserfromdeleted); - $this->assertEmpty($message3->timeusertodeleted); + $this->assertEquals($user1->id, $mua3->userid); + $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); } /** @@ -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); - $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); - $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); - $this->send_message($user2, $user1, 'Word.', 0, $time + 3); + $m1id = $this->send_message($user1, $user2, 'Yo!', 0, $time); + $m2id = $this->send_message($user2, $user1, 'Sup mang?', 0, $time + 1); + $m3id = $this->send_message($user1, $user2, 'Writing PHPUnit tests!', 0, $time + 2); + $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'); - $this->assertCount(4, $messages); + $muas = $DB->get_records('message_user_actions', array(), 'timecreated ASC'); + $this->assertCount(4, $muas); + // Sort by id. + ksort($muas); - $message1 = array_shift($messages); - $message2 = array_shift($messages); - $message3 = array_shift($messages); - $message4 = array_shift($messages); + $mua1 = array_shift($muas); + $mua2 = array_shift($muas); + $mua3 = array_shift($muas); + $mua4 = array_shift($muas); - $this->assertNotEmpty($message1->timeuserfromdeleted); - $this->assertEmpty($message1->timeusertodeleted); + $this->assertEquals($user1->id, $mua1->userid); + $this->assertEquals($m1id, $mua1->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua1->action); - $this->assertEmpty($message2->timeuserfromdeleted); - $this->assertNotEmpty($message2->timeusertodeleted); + $this->assertEquals($user1->id, $mua2->userid); + $this->assertEquals($m2id, $mua2->messageid); + $this->assertEquals(\core_message\api::MESSAGE_ACTION_DELETED, $mua2->action); - $this->assertNotEmpty($message3->timeuserfromdeleted); - $this->assertEmpty($message3->timeusertodeleted); + $this->assertEquals($user1->id, $mua3->userid); + $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); } /** diff --git a/message/tests/messagelib_test.php b/message/tests/messagelib_test.php index e08bd830b4a..a0536eaa55c 100644 --- a/message/tests/messagelib_test.php +++ b/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. - $this->send_fake_message($userfrom1, $userto, 'Notification', 1); - $this->send_fake_message($userfrom2, $userto, 'Notification', 1); + // Mark message as read. + \core_message\api::mark_message_as_read($userto->id, $messageid); - // Should only count the messages. - $this->assertEquals(2, message_count_unread_messages($userto)); - $this->assertEquals(1, message_count_unread_messages($userto, $userfrom1)); + // Should only count the messages that weren't read by the current user. + $this->assertEquals(1, message_count_unread_messages($userto)); + $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)); - message_delete_message($message, $userto->id); + \core_message\api::delete_message($userto->id, $messageid); // Should only count the messages that weren't deleted by the current user. $this->assertEquals(1, message_count_unread_messages($userto)); diff --git a/message/tests/search_received_test.php b/message/tests/search_received_test.php index 9eeb93e2e13..fcc29215a9d 100644 --- a/message/tests/search_received_test.php +++ b/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); diff --git a/message/tests/search_sent_test.php b/message/tests/search_sent_test.php index 9fe45c51aad..69338d1f9d9 100644 --- a/message/tests/search_sent_test.php +++ b/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);