From eb5865da8bfdb4731b343b6302d496bc36448b69 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Mon, 22 Oct 2018 11:48:20 +0800 Subject: [PATCH 1/3] MDL-63549 core_message: add group support to api::get_conversations - Added support for returning group conversations - Added support for conversation type filtering - Added support for NO favourites, or ONLY favourites restrictions. --- message/classes/api.php | 319 ++++++++++++++++-------- message/classes/helper.php | 7 +- message/tests/api_test.php | 484 ++++++++++++++++++++++++------------- 3 files changed, 539 insertions(+), 271 deletions(-) diff --git a/message/classes/api.php b/message/classes/api.php index ee60a6bf69c..08243942a69 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -275,6 +275,50 @@ class api { return array($contacts, $courses, $noncontacts); } + /** + * Gets the subnames for any conversations linked to components. + * + * The subname is like a subtitle for the conversation, to compliment it's name. + * + * @param array $conversations a list of conversations records. + * @return array the array of subnames, index by conversation id. + */ + protected static function get_linked_conversation_subnames(array $conversations) { + global $DB; + + $linkedconversations = []; + foreach ($conversations as $conversation) { + if (!is_null($conversation->component) && !is_null($conversation->itemtype)) { + $linkedconversations[$conversation->component][$conversation->itemtype][$conversation->id] + = $conversation->itemid; + } + } + if (empty($linkedconversations)) { + return []; + } + + // TODO: MDL-63814: Working out the subname for linked conversations should be done in a generic way. + // Get the itemid, but only for course group linked conversation for now. + $convsubnames = []; + if (!empty($linkeditems = $linkedconversations['core_group']['groups'])) { // Format: [conversationid => itemid]. + // Get the name of the course to which the group belongs. + list ($groupidsql, $groupidparams) = $DB->get_in_or_equal(array_values($linkeditems), SQL_PARAMS_NAMED, 'groupid'); + $sql = "SELECT g.id, c.shortname + FROM {groups} g + JOIN {course} c + ON g.courseid = c.id + WHERE g.id $groupidsql"; + $courseinfo = $DB->get_records_sql($sql, $groupidparams); + foreach ($linkeditems as $convid => $groupid) { + if (array_key_exists($groupid, $courseinfo)) { + $convsubnames[$convid] = format_string($courseinfo[$groupid]->shortname); + } + } + } + return $convsubnames; + } + + /** * Returns the contacts and their conversation to display in the contacts area. * @@ -296,36 +340,58 @@ class api { * @param int $userid The user id * @param int $limitfrom * @param int $limitnum - * @param int $type the conversation type. - * @param bool $favouritesonly whether to retrieve only the favourite conversations for the user, or not. - * @return array + * @param int $type the type of the conversation, if you wish to filter to a certain type (see api constants). + * @param bool $favourites whether to include NO favourites (false) or ONLY favourites (true), or null to ignore this setting. + * @return array the array of conversations + * @throws \moodle_exception */ public static function get_conversations($userid, $limitfrom = 0, $limitnum = 20, int $type = null, - bool $favouritesonly = false) { + bool $favourites = null) { global $DB; - $favouritesql = ""; - $favouriteparams = []; - if ($favouritesonly) { - // Ask the favourites subsystem for the user's favourite conversations. - $service = \core_favourites\service_factory::get_service_for_user_context(\context_user::instance($userid)); - $favourites = $service->find_favourites_by_type('core_message', 'message_conversations'); - if (empty($favourites)) { - return []; // No favourited conversations, so return none. - } - $favids = array_values(array_map(function ($fav) { - return $fav->itemid; - }, $favourites)); - list ($insql, $inparams) = $DB->get_in_or_equal($favids, SQL_PARAMS_NAMED, 'favouriteids'); - $favouritesql = " AND m.conversationid {$insql} "; - $favouriteparams = $inparams; + if (!is_null($type) && !in_array($type, [self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + self::MESSAGE_CONVERSATION_TYPE_GROUP])) { + throw new \moodle_exception("Invalid value ($type) for type param, please see api constants."); } - // 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 + // We need to know which conversations are favourites, so we can either: + // 1) Include the 'isfavourite' attribute on conversations (when $favourite = null and we're including all conversations) + // 2) Restrict the results to ONLY those conversations which are favourites (when $favourite = true) + // 3) Restrict the results to ONLY those conversations which are NOT favourites (when $favourite = false). + $service = \core_favourites\service_factory::get_service_for_user_context(\context_user::instance($userid)); + $favouriteconversations = $service->find_favourites_by_type('core_message', 'message_conversations'); + $favouriteconversationids = array_column($favouriteconversations, 'itemid'); + if ($favourites && empty($favouriteconversationids)) { + return []; // If we are aiming to return ONLY favourites, and we have none, there's nothing more to do. + } + + // CONVERSATIONS AND MOST RECENT MESSAGE. + // Include those conversations with messages first (ordered by most recent message, desc), then add any conversations which + // don't have messages, such as newly created group conversations. + // Because we're sorting by message 'timecreated', those conversations without messages could be at either the start or the + // end of the results (behaviour for sorting of nulls differs between DB vendors), so we use the case to presort these. + + // If we need to return ONLY favourites, or NO favourites, generate the SQL snippet. + $favouritesql = ""; + $favouriteparams = []; + if (is_bool($favourites)) { + if (!empty($favouriteconversationids)) { + list ($insql, $inparams) = $DB->get_in_or_equal($favouriteconversationids, SQL_PARAMS_NAMED, 'favouriteids'); + $favouritesql = $favourites ? " AND mc.id {$insql} " : " AND mc.id NOT {$insql} "; + $favouriteparams = $inparams; + } + } + + // If we need to restrict type, generate the SQL snippet. + $typesql = !is_null($type) ? " AND mc.type = :convtype " : ""; + + $sql = "SELECT m.id as messageid, mc.id as id, mc.name as conversationname, mc.type as conversationtype, m.useridfrom, + m.smallmessage, m.timecreated, mc.component, mc.itemtype, mc.itemid + FROM {message_conversations} mc + INNER JOIN {message_conversation_members} mcm + ON (mcm.conversationid = mc.id AND mcm.userid = :userid3) + LEFT JOIN ( + SELECT m.conversationid, MAX(m.id) AS messageid FROM {messages} m INNER JOIN ( SELECT m.conversationid, MAX(m.timecreated) as maxtime @@ -341,61 +407,124 @@ class api { 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 $favouritesql - ORDER BY m.timecreated DESC"; + ON lastmessage.conversationid = mc.id + LEFT JOIN {messages} m + ON m.id = lastmessage.messageid + WHERE mc.id IS NOT NULL $typesql $favouritesql + ORDER BY (CASE WHEN m.timecreated IS NULL THEN 0 ELSE 1 END) DESC, m.timecreated DESC, id DESC"; $params = array_merge($favouriteparams, ['userid' => $userid, 'action' => self::MESSAGE_ACTION_DELETED, - 'userid2' => $userid]); - $messageset = $DB->get_recordset_sql($sql, $params, $limitfrom, $limitnum); + 'userid2' => $userid, 'userid3' => $userid, 'convtype' => $type]); + $conversationset = $DB->get_recordset_sql($sql, $params, $limitfrom, $limitnum); - $messages = []; - foreach ($messageset as $message) { - $messages[$message->id] = $message; + $conversations = []; + $uniquemembers = []; + $members = []; + foreach ($conversationset as $conversation) { + $conversations[] = $conversation; + $members[$conversation->id] = []; } - $messageset->close(); + $conversationset->close(); - // If there are no messages return early. - if (empty($messages)) { + // If there are no conversations found, then return early. + if (empty($conversations)) { return []; } - // We need to pull out the list of other users that are part of each of these conversations. This + // COMPONENT-LINKED CONVERSATION SUBNAME. + // This subname will vary, depending on the component which created the linked conversation. + // For now, this is ONLY course groups. + $convsubnames = self::get_linked_conversation_subnames($conversations); + + // MEMBERS. + // Ideally, we want to get 1 member for each conversation, but this depends on the type and whether there is a recent + // message or not. + // + // For 'individual' type conversations between 2 users, regardless of who sent the last message, + // we want the details of the other member in the conversation (i.e. not the current user). + // + // For 'group' type conversations, we want the details of the member who sent the last message, if there is one. + // This can be the current user or another group member, but for groups without messages, this will be empty. + // + // This also means that if type filtering is specified and only group conversations are returned, we don't need this extra + // query to get the 'other' user as we already have that information. + + // Work out which members we have already, and which ones we might need to fetch. + // If all the last messages were from another user, then we don't need to fetch anything further. + foreach ($conversations as $conversation) { + if ($conversation->conversationtype == self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL) { + if (!is_null($conversation->useridfrom) && $conversation->useridfrom != $userid) { + $members[$conversation->id][$conversation->useridfrom] = $conversation->useridfrom; + $uniquemembers[$conversation->useridfrom] = $conversation->useridfrom; + } else { + $individualconversations[] = $conversation->id; + } + } else if ($conversation->conversationtype == self::MESSAGE_CONVERSATION_TYPE_GROUP) { + // If we have a recent message, the sender is our member. + if (!is_null($conversation->useridfrom)) { + $members[$conversation->id][$conversation->useridfrom] = $conversation->useridfrom; + $uniquemembers[$conversation->useridfrom] = $conversation->useridfrom; + } + } + } + // If we need to fetch any member information for any of the individual conversations. + // This is the case if any of the individual conversations have a recent message sent by the current user. + if (!empty($individualconversations)) { + list ($icidinsql, $icidinparams) = $DB->get_in_or_equal($individualconversations, SQL_PARAMS_NAMED, 'convid'); + $indmembersql = "SELECT mcm.id, mcm.conversationid, mcm.userid + FROM {message_conversation_members} mcm + WHERE mcm.conversationid $icidinsql + AND mcm.userid != :userid + ORDER BY mcm.id"; + $indmemberparams = array_merge($icidinparams, ['userid' => $userid]); + $conversationmembers = $DB->get_records_sql($indmembersql, $indmemberparams); + + foreach ($conversationmembers as $mid => $member) { + $members[$member->conversationid][$member->userid] = $member->userid; + $uniquemembers[$member->userid] = $member->userid; + } + } + $memberids = array_values($uniquemembers); + + // We could fail early here if we're sure that: + // a) we have no otherusers for all the conversations (users may have been deleted) + // b) we're sure that all conversations are individual (1:1). + + // We need to pull out the list of users info corresponding to the memberids in the 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 // performance (typically due to join buffer exhaustion). - $otheruserids = array_map(function($message) use ($userid) { - return ($message->useridfrom == $userid) ? $message->useridto : $message->useridfrom; - }, array_values($messages)); + if (!empty($memberids)) { + $memberinfo = helper::get_member_info($userid, $memberids); - // Ok, let's get the other members in the conversations. - list($useridsql, $usersparams) = $DB->get_in_or_equal($otheruserids); - $userfields = \user_picture::fields('u', array('lastaccess')); - $userssql = "SELECT $userfields - FROM {user} u - WHERE id $useridsql - AND deleted = 0"; - $otherusers = $DB->get_records_sql($userssql, $usersparams); - - // If there are no other users (user may have been deleted), then do not continue. - if (empty($otherusers)) { - return []; + // Update the members array with the member information. + $deletedmembers = []; + foreach ($members as $convid => $memberarr) { + foreach ($memberarr as $key => $memberid) { + if (array_key_exists($memberid, $memberinfo)) { + // If the user is deleted, remember that. + if ($memberinfo[$memberid]->isdeleted) { + $deletedmembers[$convid][] = $memberid; + } + $members[$convid][$key] = $memberinfo[$memberid]; + } + } + } } - $contactssql = "SELECT contactid - FROM {message_contacts} - WHERE userid = ? - AND contactid $useridsql"; - $contacts = $DB->get_records_sql($contactssql, array_merge([$userid], $usersparams)); + // MEMBER COUNT. + $cids = array_column($conversations, 'id'); + list ($cidinsql, $cidinparams) = $DB->get_in_or_equal($cids, SQL_PARAMS_NAMED, 'convid'); + $membercountsql = "SELECT conversationid, count(id) AS membercount + FROM {message_conversation_members} mcm + WHERE mcm.conversationid $cidinsql + GROUP BY mcm.conversationid"; + $membercounts = $DB->get_records_sql($membercountsql, $cidinparams); - // Finally, let's get the unread messages count for this user so that we can add them + // UNREAD MESSAGE COUNT. + // Finally, let's get the unread messages count for this user so that we can add it // to the conversation. Remember we need to ignore the messages the user sent. - $unreadcountssql = 'SELECT m.useridfrom, count(m.id) as count + $unreadcountssql = 'SELECT m.conversationid, count(m.id) as unreadcount FROM {messages} m INNER JOIN {message_conversations} mc ON mc.id = m.conversationid @@ -407,49 +536,45 @@ class api { WHERE mcm.userid = ? AND m.useridfrom != ? AND mua.id is NULL - GROUP BY useridfrom'; + GROUP BY m.conversationid'; $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(); - 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)) { + // Now, create the final return structure. + $arrconversations = []; + foreach ($conversations as $conversation) { + // It's possible other users have been deleted. + // In cases like this, we still want to include the conversation if it's of type 'group'. + // Individual conversations are skipped if the other member has been deleted. + if (isset($deletedmembers[$conversation->id]) && + $conversation->conversationtype == self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL) { continue; } - // Add the other user's information to the conversation, if we have one. - foreach ($userproperties as $prop) { - $conversation->$prop = ($otheruser) ? $otheruser->$prop : null; + $conv = new \stdClass(); + $conv->id = $conversation->id; + $conv->name = $conversation->conversationname; + $conv->subname = $convsubnames[$conv->id] ?? null; + $conv->type = $conversation->conversationtype; + $conv->membercount = $membercounts[$conv->id]->membercount; + $conv->isfavourite = in_array($conv->id, $favouriteconversationids); + $conv->isread = isset($unreadcounts[$conv->id]) ? false : true; + $conv->unreadcount = isset($unreadcounts[$conv->id]) ? $unreadcounts[$conv->id]->unreadcount : null; + $conv->members = $members[$conv->id]; + + // Add the most recent message information. + $conv->messages = []; + if ($conversation->smallmessage) { + $msg = new \stdClass(); + $msg->id = $conversation->messageid; + $msg->text = clean_param($conversation->smallmessage, PARAM_NOTAGS); + $msg->useridfrom = $conversation->useridfrom; + $msg->timecreated = $conversation->timecreated; + $conv->messages[] = $msg; } - // Add the contact's information, if we have one. - $conversation->blocked = ($contact) ? $contact->blocked : null; - - // Add the message information. - $conversation->messageid = $message->id; - $conversation->smallmessage = $message->smallmessage; - $conversation->useridfrom = $message->useridfrom; - - // Only consider it unread if $user has unread messages. - if (isset($unreadcounts[$otheruserid])) { - $conversation->isread = false; - $conversation->unreadcount = $unreadcounts[$otheruserid]->count; - } else { - $conversation->isread = true; - } - - $arrconversations[$otheruserid] = helper::create_contact($conversation); + $arrconversations[] = $conv; } - return $arrconversations; } diff --git a/message/classes/helper.php b/message/classes/helper.php index 3b090757de0..a1ce9eb4bf1 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -491,14 +491,13 @@ class helper { list($useridsql, $usersparams) = $DB->get_in_or_equal($userids); $userfields = \user_picture::fields('u', array('lastaccess')); - $userssql = "SELECT $userfields, mc.id AS contactid, mub.id AS blockedid + $userssql = "SELECT $userfields, u.deleted, mc.id AS contactid, mub.id AS blockedid FROM {user} u LEFT JOIN {message_contacts} mc ON (mc.userid = ? AND mc.contactid = u.id) LEFT JOIN {message_users_blocked} mub ON (mub.userid = ? AND mub.blockeduserid = u.id) - WHERE u.id $useridsql - AND u.deleted = 0"; + WHERE u.id $useridsql"; $usersparams = array_merge([$referenceuserid, $referenceuserid], $usersparams); $otherusers = $DB->get_records_sql($userssql, $usersparams); @@ -524,6 +523,8 @@ class helper { $data->iscontact = ($member->contactid) ? true : false; $data->isblocked = ($member->blockedid) ? true : false; + $data->isdeleted = ($member->deleted) ? true : false; + $members[$data->id] = $data; } return $members; diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 2b9cbc4b78e..d75284306ab 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -488,7 +488,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { // Confirm the conversation is from the non-deleted user. $conversation = reset($conversations); - $this->assertEquals($user3->id, $conversation->userid); + $this->assertEquals($convoids[1], $conversation->id); } /** @@ -642,183 +642,366 @@ class core_message_api_testcase extends core_message_messagelib_testcase { } /** - * Tests retrieving conversations. + * Helper to seed the database with initial state. */ - public function test_get_conversations() { + protected function create_conversation_test_data() { // Create some users. $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); $user3 = self::getDataGenerator()->create_user(); $user4 = self::getDataGenerator()->create_user(); - // The person doing the search. - $this->setUser($user1); + $time = 1; - // No conversations yet. + // Create some conversations. We want: + // 1) At least one of each type (group, individual) of which user1 IS a member and DID send the most recent message. + // 2) At least one of each type (group, individual) of which user1 IS a member and DID NOT send the most recent message. + // 3) At least one of each type (group, individual) of which user1 IS NOT a member. + // 4) At least two group conversation having 0 messages, of which user1 IS a member (To confirm conversationid ordering). + // 5) At least one group conversation having 0 messages, of which user1 IS NOT a member. + + // Individual conversation, user1 is a member, last message from other user. + $ic1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$user1->id, $user2->id]); + testhelper::send_fake_message_to_conversation($user1, $ic1->id, 'Message 1', $time); + testhelper::send_fake_message_to_conversation($user2, $ic1->id, 'Message 2', $time + 1); + + // Individual conversation, user1 is a member, last message from user1. + $ic2 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$user1->id, $user3->id]); + testhelper::send_fake_message_to_conversation($user3, $ic2->id, 'Message 3', $time + 2); + testhelper::send_fake_message_to_conversation($user1, $ic2->id, 'Message 4', $time + 3); + + // Individual conversation, user1 is not a member. + $ic3 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$user2->id, $user3->id]); + testhelper::send_fake_message_to_conversation($user2, $ic3->id, 'Message 5', $time + 4); + testhelper::send_fake_message_to_conversation($user3, $ic3->id, 'Message 6', $time + 5); + + // Group conversation, user1 is not a member. + $gc1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user2->id, $user3->id, $user4->id], 'Project discussions'); + testhelper::send_fake_message_to_conversation($user2, $gc1->id, 'Message 7', $time + 6); + testhelper::send_fake_message_to_conversation($user4, $gc1->id, 'Message 8', $time + 7); + + // Group conversation, user1 is a member, last message from another user. + $gc2 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user1->id, $user3->id, $user4->id], 'Group chat'); + testhelper::send_fake_message_to_conversation($user1, $gc2->id, 'Message 9', $time + 8); + testhelper::send_fake_message_to_conversation($user3, $gc2->id, 'Message 10', $time + 9); + testhelper::send_fake_message_to_conversation($user4, $gc2->id, 'Message 11', $time + 10); + + // Group conversation, user1 is a member, last message from user1. + $gc3 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user1->id, $user2->id, $user3->id, $user4->id], 'Group chat again!'); + testhelper::send_fake_message_to_conversation($user4, $gc3->id, 'Message 12', $time + 11); + testhelper::send_fake_message_to_conversation($user3, $gc3->id, 'Message 13', $time + 12); + testhelper::send_fake_message_to_conversation($user1, $gc3->id, 'Message 14', $time + 13); + + // Empty group conversations (x2), user1 is a member. + $gc4 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user1->id, $user2->id, $user3->id], 'Empty group'); + $gc5 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user1->id, $user2->id, $user4->id], 'Another empty group'); + + // Empty group conversation, user1 is NOT a member. + $gc6 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user2->id, $user3->id, $user4->id], 'Empty group 3'); + + return [$user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, $gc1, $gc2, $gc3, $gc4, $gc5, $gc6]; + } + + /** + * Test verifying get_conversations when no limits, offsets, type filters or favourite restrictions are used. + */ + public function test_get_conversations_no_restrictions() { + // No conversations should exist yet. + $user1 = self::getDataGenerator()->create_user(); $this->assertEquals([], \core_message\api::get_conversations($user1->id)); - // Send some messages back and forth, have some different conversations with different users. - $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); - $messageid1 = $this->send_fake_message($user2, $user1, 'Word.', 0, $time + 4); + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); - $this->send_fake_message($user1, $user3, 'Booyah', 0, $time + 5); - $this->send_fake_message($user3, $user1, 'Whaaat?', 0, $time + 6); - $this->send_fake_message($user1, $user3, 'Nothing.', 0, $time + 7); - $messageid2 = $this->send_fake_message($user3, $user1, 'Cool.', 0, $time + 8); + // Get all conversations for user1. + $conversations = core_message\api::get_conversations($user1->id); - $this->send_fake_message($user1, $user4, 'Hey mate, you see the new messaging UI in Moodle?', 0, $time + 9); - $this->send_fake_message($user4, $user1, 'Yah brah, it\'s pretty rad.', 0, $time + 10); - $messageid3 = $this->send_fake_message($user1, $user4, 'Dope.', 0, $time + 11); + // Verify there are 2 individual conversation, 2 group conversations, and 2 empty group conversations. + // The conversations with the most recent messages should be listed first, followed by the empty + // conversations, with the most recently created first. + $this->assertCount(6, $conversations); + $typecounts = array_count_values(array_column($conversations, 'type')); + $this->assertEquals(2, $typecounts[1]); + $this->assertEquals(4, $typecounts[2]); - // Retrieve the conversations. - $conversations = \core_message\api::get_conversations($user1->id); + // Those conversations having messages should be listed first, ordered by most recent message time. + $this->assertEquals($gc3->id, $conversations[0]->id); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $conversations[0]->type); + $this->assertFalse($conversations[0]->isfavourite); + $this->assertCount(1, $conversations[0]->members); + $this->assertEquals(4, $conversations[0]->membercount); + $this->assertCount(1, $conversations[0]->messages); + $this->assertEquals("Message 14", $conversations[0]->messages[0]->text); + $this->assertEquals($user1->id, $conversations[0]->messages[0]->useridfrom); - // Confirm the data is correct. - $this->assertEquals(3, count($conversations)); + $this->assertEquals($gc2->id, $conversations[1]->id); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $conversations[1]->type); + $this->assertFalse($conversations[1]->isfavourite); + $this->assertCount(1, $conversations[1]->members); + $this->assertEquals(3, $conversations[1]->membercount); + $this->assertCount(1, $conversations[1]->messages); + $this->assertEquals("Message 11", $conversations[1]->messages[0]->text); + $this->assertEquals($user4->id, $conversations[1]->messages[0]->useridfrom); - $message1 = array_shift($conversations); - $message2 = array_shift($conversations); - $message3 = array_shift($conversations); + $this->assertEquals($ic2->id, $conversations[2]->id); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $conversations[2]->type); + $this->assertFalse($conversations[2]->isfavourite); + $this->assertCount(1, $conversations[2]->members); + $this->assertEquals($user3->id, $conversations[2]->members[$user3->id]->id); + $this->assertEquals(2, $conversations[2]->membercount); + $this->assertCount(1, $conversations[2]->messages); + $this->assertEquals("Message 4", $conversations[2]->messages[0]->text); + $this->assertEquals($user1->id, $conversations[2]->messages[0]->useridfrom); - $this->assertEquals($user4->id, $message1->userid); - $this->assertEquals($user1->id, $message1->useridfrom); - $this->assertTrue($message1->ismessaging); - $this->assertEquals('Dope.', $message1->lastmessage); - $this->assertEquals($messageid3, $message1->messageid); - $this->assertNull($message1->isonline); - $this->assertFalse($message1->isread); - $this->assertFalse($message1->isblocked); - $this->assertEquals(1, $message1->unreadcount); + $this->assertEquals($ic1->id, $conversations[3]->id); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $conversations[3]->type); + $this->assertFalse($conversations[3]->isfavourite); + $this->assertCount(1, $conversations[3]->members); + $this->assertEquals(2, $conversations[3]->membercount); + $this->assertCount(1, $conversations[3]->messages); + $this->assertEquals("Message 2", $conversations[3]->messages[0]->text); + $this->assertEquals($user2->id, $conversations[3]->messages[0]->useridfrom); - $this->assertEquals($user3->id, $message2->userid); - $this->assertEquals($user3->id, $message2->useridfrom); - $this->assertTrue($message2->ismessaging); - $this->assertEquals('Cool.', $message2->lastmessage); - $this->assertEquals($messageid2, $message2->messageid); - $this->assertNull($message2->isonline); - $this->assertFalse($message2->isread); - $this->assertFalse($message2->isblocked); - $this->assertEquals(2, $message2->unreadcount); + // Of the groups without messages, we expect to see the most recently created first. + $this->assertEquals($gc5->id, $conversations[4]->id); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $conversations[4]->type); + $this->assertFalse($conversations[4]->isfavourite); + $this->assertCount(0, $conversations[4]->members); // No members returned, because no recent messages exist. + $this->assertEquals(3, $conversations[4]->membercount); + $this->assertEmpty($conversations[4]->messages); - $this->assertEquals($user2->id, $message3->userid); - $this->assertEquals($user2->id, $message3->useridfrom); - $this->assertTrue($message3->ismessaging); - $this->assertEquals('Word.', $message3->lastmessage); - $this->assertEquals($messageid1, $message3->messageid); - $this->assertNull($message3->isonline); - $this->assertFalse($message3->isread); - $this->assertFalse($message3->isblocked); - $this->assertEquals(2, $message3->unreadcount); + $this->assertEquals($gc4->id, $conversations[5]->id); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $conversations[5]->type); + $this->assertFalse($conversations[5]->isfavourite); + $this->assertCount(0, $conversations[5]->members); + $this->assertEquals(3, $conversations[5]->membercount); + $this->assertEmpty($conversations[5]->messages); + + // Verify format of the return structure. + foreach ($conversations as $conv) { + $this->assertObjectHasAttribute('id', $conv); + $this->assertObjectHasAttribute('name', $conv); + $this->assertObjectHasAttribute('subname', $conv); + $this->assertObjectHasAttribute('type', $conv); + $this->assertObjectHasAttribute('isfavourite', $conv); + $this->assertObjectHasAttribute('membercount', $conv); + $this->assertObjectHasAttribute('isread', $conv); + $this->assertObjectHasAttribute('unreadcount', $conv); + $this->assertObjectHasAttribute('members', $conv); + foreach ($conv->members as $member) { + $this->assertObjectHasAttribute('id', $member); + $this->assertObjectHasAttribute('fullname', $member); + $this->assertObjectHasAttribute('profileimageurl', $member); + $this->assertObjectHasAttribute('profileimageurlsmall', $member); + $this->assertObjectHasAttribute('isonline', $member); + $this->assertObjectHasAttribute('showonlinestatus', $member); + $this->assertObjectHasAttribute('isblocked', $member); + $this->assertObjectHasAttribute('iscontact', $member); + } + $this->assertObjectHasAttribute('messages', $conv); + foreach ($conv->messages as $message) { + $this->assertObjectHasAttribute('id', $message); + $this->assertObjectHasAttribute('useridfrom', $message); + $this->assertObjectHasAttribute('text', $message); + $this->assertObjectHasAttribute('timecreated', $message); + } + } } /** * Tests retrieving conversations with a limit and offset to ensure pagination works correctly. */ public function test_get_conversations_limit_offset() { - // Create some users. - $user1 = self::getDataGenerator()->create_user(); - $user2 = self::getDataGenerator()->create_user(); - $user3 = self::getDataGenerator()->create_user(); - $user4 = self::getDataGenerator()->create_user(); + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); - // The person doing the search. - $this->setUser($user1); + // Get all conversations for user1, limited to 1 result. + $conversations = core_message\api::get_conversations($user1->id, 0, 1); - // Send some messages back and forth, have some different conversations with different users. - $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); - $messageid1 = $this->send_fake_message($user2, $user1, 'Word.', 0, $time + 4); + // Verify the first conversation. + $this->assertCount(1, $conversations); + $conversation = array_shift($conversations); + $this->assertEquals($conversation->id, $gc3->id); - $this->send_fake_message($user1, $user3, 'Booyah', 0, $time + 5); - $this->send_fake_message($user3, $user1, 'Whaaat?', 0, $time + 6); - $this->send_fake_message($user1, $user3, 'Nothing.', 0, $time + 7); - $messageid2 = $this->send_fake_message($user3, $user1, 'Cool.', 0, $time + 8); - - $this->send_fake_message($user1, $user4, 'Hey mate, you see the new messaging UI in Moodle?', 0, $time + 9); - $this->send_fake_message($user4, $user1, 'Yah brah, it\'s pretty rad.', 0, $time + 10); - $messageid3 = $this->send_fake_message($user1, $user4, 'Dope.', 0, $time + 11); - - // Retrieve the conversations. + // Verify the next conversation. $conversations = \core_message\api::get_conversations($user1->id, 1, 1); - - // We should only have one conversation because of the limit. $this->assertCount(1, $conversations); + $this->assertEquals($gc2->id, $conversations[0]->id); - $conversation = array_shift($conversations); - - $this->assertEquals($user3->id, $conversation->userid); - $this->assertEquals($user3->id, $conversation->useridfrom); - $this->assertTrue($conversation->ismessaging); - $this->assertEquals('Cool.', $conversation->lastmessage); - $this->assertEquals($messageid2, $conversation->messageid); - $this->assertNull($conversation->isonline); - $this->assertFalse($conversation->isread); - $this->assertFalse($conversation->isblocked); - $this->assertEquals(2, $conversation->unreadcount); - - // Retrieve the next conversation. + // Verify the next conversation. $conversations = \core_message\api::get_conversations($user1->id, 2, 1); - - // We should only have one conversation because of the limit. $this->assertCount(1, $conversations); + $this->assertEquals($ic2->id, $conversations[0]->id); - $conversation = array_shift($conversations); + // Skip one and get both empty conversations. + $conversations = \core_message\api::get_conversations($user1->id, 4, 2); + $this->assertCount(2, $conversations); + $this->assertEquals($gc5->id, $conversations[0]->id); + $this->assertEmpty($conversations[0]->messages); + $this->assertEquals($gc4->id, $conversations[1]->id); + $this->assertEmpty($conversations[1]->messages); - $this->assertEquals($user2->id, $conversation->userid); - $this->assertEquals($user2->id, $conversation->useridfrom); - $this->assertTrue($conversation->ismessaging); - $this->assertEquals('Word.', $conversation->lastmessage); - $this->assertEquals($messageid1, $conversation->messageid); - $this->assertNull($conversation->isonline); - $this->assertFalse($conversation->isread); - $this->assertFalse($conversation->isblocked); - $this->assertEquals(2, $conversation->unreadcount); - - // Ask for an offset that doesn't exist. - $conversations = \core_message\api::get_conversations($user1->id, 4, 1); - - // We should not get any conversations back. + // Ask for an offset that doesn't exist and verify no conversations are returned. + $conversations = \core_message\api::get_conversations($user1->id, 10, 1); $this->assertCount(0, $conversations); } + /** + * Test verifying the type filtering behaviour of the + */ + public function test_get_conversations_type_filter() { + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // Verify we can ask for only individual conversations. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL); + $this->assertCount(2, $conversations); + + // Verify we can ask for only group conversations. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP); + $this->assertCount(4, $conversations); + + // Verify an exception is thrown if an unrecognized type is specified. + $this->expectException(\moodle_exception::class); + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, 0); + } + /** * Tests retrieving conversations when a conversation contains a deleted user. */ public function test_get_conversations_with_deleted_user() { + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // Delete the second user and retrieve the conversations. + // We should have 5, as $ic1 drops off the list. + // Group conversations remain albeit with less members. + delete_user($user2); + $conversations = \core_message\api::get_conversations($user1->id); + $this->assertCount(5, $conversations); + $this->assertEquals($gc3->id, $conversations[0]->id); + $this->assertcount(1, $conversations[0]->members); + $this->assertEquals($gc2->id, $conversations[1]->id); + $this->assertcount(1, $conversations[1]->members); + $this->assertEquals($ic2->id, $conversations[2]->id); + $this->assertEquals($gc5->id, $conversations[3]->id); + $this->assertEquals($gc4->id, $conversations[4]->id); + + // Delete a user from a group conversation where that user had sent the most recent message. + // This user will still be present in the members array, as will the message in the messages array. + delete_user($user4); + $conversations = \core_message\api::get_conversations($user1->id); + $this->assertCount(5, $conversations); + $this->assertEquals($gc2->id, $conversations[1]->id); + $this->assertcount(1, $conversations[1]->members); + $this->assertEquals($user4->id, $conversations[1]->members[$user4->id]->id); + $this->assertcount(1, $conversations[1]->messages); + $this->assertEquals($user4->id, $conversations[1]->messages[0]->useridfrom); + + // Delete the third user and retrieve the conversations. + // We should have 4, as $ic1, $ic2 drop off the list. + // Group conversations remain albeit with less members. + delete_user($user3); + $conversations = \core_message\api::get_conversations($user1->id); + $this->assertCount(4, $conversations); + $this->assertEquals($gc3->id, $conversations[0]->id); + $this->assertcount(1, $conversations[0]->members); + $this->assertEquals($gc2->id, $conversations[1]->id); + $this->assertcount(1, $conversations[1]->members); + $this->assertEquals($gc5->id, $conversations[2]->id); + $this->assertEquals($gc4->id, $conversations[3]->id); + } + + /** + * Test verifying the behaviour of get_conversations() when fetching favourite conversations. + */ + public function test_get_conversations_favourite_conversations() { + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // Try to get ONLY favourite conversations, when no favourites exist. + $this->assertEquals([], \core_message\api::get_conversations($user1->id, 0, 20, null, true)); + + // Try to get NO favourite conversations, when no favourites exist. + $this->assertCount(6, \core_message\api::get_conversations($user1->id, 0, 20, null, false)); + + // Mark a few conversations as favourites. + \core_message\api::set_favourite_conversation($ic1->id, $user1->id); + \core_message\api::set_favourite_conversation($gc2->id, $user1->id); + \core_message\api::set_favourite_conversation($gc5->id, $user1->id); + + // Get the conversations, first with no restrictions, confirming the favourite status of the conversations. + $conversations = \core_message\api::get_conversations($user1->id); + $this->assertCount(6, $conversations); + foreach ($conversations as $conv) { + if (in_array($conv->id, [$ic1->id, $gc2->id, $gc5->id])) { + $this->assertTrue($conv->isfavourite); + } + } + + // Now, get ONLY favourite conversations. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, null, true); + $this->assertCount(3, $conversations); + foreach ($conversations as $conv) { + $this->assertTrue($conv->isfavourite); + } + + // Now, try ONLY favourites of type 'group'. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, true); + $this->assertCount(2, $conversations); + foreach ($conversations as $conv) { + $this->assertTrue($conv->isfavourite); + } + + // And NO favourite conversations. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, null, false); + $this->assertCount(3, $conversations); + foreach ($conversations as $conv) { + $this->assertFalse($conv->isfavourite); + } + } + + /** + * Test verifying that group linked conversations are returned and contain a subname matching the course name. + */ + public function test_get_conversations_group_linked() { // Create some users. $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); $user3 = self::getDataGenerator()->create_user(); - // Send some messages back and forth, have some different conversations with different users. - $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); + $course1 = $this->getDataGenerator()->create_course(); - $this->send_fake_message($user1, $user3, 'Booyah', 0, $time + 5); - $this->send_fake_message($user3, $user1, 'Whaaat?', 0, $time + 6); - $this->send_fake_message($user1, $user3, 'Nothing.', 0, $time + 7); - $this->send_fake_message($user3, $user1, 'Cool.', 0, $time + 8); + // Create a group with a linked conversation. + $this->setAdminUser(); + $this->getDataGenerator()->enrol_user($user1->id, $course1->id); + $this->getDataGenerator()->enrol_user($user2->id, $course1->id); + $this->getDataGenerator()->enrol_user($user3->id, $course1->id); + $group1 = $this->getDataGenerator()->create_group(array('courseid' => $course1->id, 'enablemessaging' => 1)); - // Delete the second user. - delete_user($user2); + // Add users to group1. + $this->getDataGenerator()->create_group_member(array('groupid' => $group1->id, 'userid' => $user1->id)); + $this->getDataGenerator()->create_group_member(array('groupid' => $group1->id, 'userid' => $user2->id)); - // Retrieve the conversations. $conversations = \core_message\api::get_conversations($user1->id); - - // We should only have one conversation because the other user was deleted. - $this->assertCount(1, $conversations); - - // Confirm the conversation is from the non-deleted user. - $conversation = reset($conversations); - $this->assertEquals($user3->id, $conversation->userid); + $this->assertEquals($course1->shortname, $conversations[0]->subname); } /** @@ -916,47 +1099,6 @@ class core_message_api_testcase extends core_message_messagelib_testcase { ), ), ), - 'Test that users with contacts and messages to self work as expected' => array( - 'users' => array( - 'user1', - 'user2', - 'user3', - ), - 'contacts' => array( - 'user1' => array( - 'user2' => 0, - 'user3' => 0, - ), - 'user2' => array( - 'user3' => 0, - ), - ), - 'messages' => array( - array( - 'from' => 'user1', - 'to' => 'user1', - 'state' => 'unread', - 'subject' => 'S1', - ), - array( - 'from' => 'user1', - 'to' => 'user1', - 'state' => 'unread', - 'subject' => 'S2', - ), - ), - 'expectations' => array( - 'user1' => array( - // User1 has conversed most recently with user1. The most recent message is S2. - array( - 'messageposition' => 0, - 'with' => 'user1', - 'subject' => 'S2', - 'unreadcount' => 0, // Messages sent to and from the same user are counted as read. - ), - ), - ), - ), 'Test conversations with a single user, where some messages are read and some are not.' => array( 'users' => array( 'user1', @@ -1219,8 +1361,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase { foreach ($data as $expectation) { $otheruser = $users[$expectation['with']]; $conversation = $conversations[$expectation['messageposition']]; - $this->assertEquals($otheruser->id, $conversation->userid); - $this->assertEquals($expectation['subject'], $conversation->lastmessage); + $this->assertEquals($otheruser->id, $conversation->members[$otheruser->id]->id); + $this->assertEquals($expectation['subject'], $conversation->messages[0]->text); $this->assertEquals($expectation['unreadcount'], $conversation->unreadcount); } } From 6f31927af8d869b56a39de318085e0bd0ba42235 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Mon, 22 Oct 2018 11:48:52 +0800 Subject: [PATCH 2/3] MDL-63549 core_message: add get_conversations web service --- lib/db/services.php | 8 + message/externallib.php | 105 +++++++++ message/tests/externallib_test.php | 339 +++++++++++++++++++++++++++++ 3 files changed, 452 insertions(+) diff --git a/lib/db/services.php b/lib/db/services.php index f9e1185f5a5..a9dcdd3d73d 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -1028,6 +1028,14 @@ $functions = array( 'type' => 'read', 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), ), + 'core_message_get_conversations' => array( + 'classname' => 'core_message_external', + 'methodname' => 'get_conversations', + 'classpath' => 'message/externallib.php', + 'description' => 'Retrieve a list of conversations for a user', + 'type' => 'read', + 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), + ), 'core_message_get_messages' => array( 'classname' => 'core_message_external', 'methodname' => 'get_messages', diff --git a/message/externallib.php b/message/externallib.php index 75c19ecf660..ec7fa2ce2f3 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -868,6 +868,34 @@ class core_message_external extends external_api { ); } + /** + * Return the structure of a conversation. + * + * @return external_single_structure + * @since Moodle 3.6 + */ + private static function get_conversation_structure() { + return new external_single_structure( + array( + 'id' => new external_value(PARAM_INT, 'The conversation id'), + 'name' => new external_value(PARAM_NOTAGS, 'The conversation name, if set', VALUE_DEFAULT, null), + 'subname' => new external_value(PARAM_NOTAGS, 'A subtitle for the conversation name, if set', VALUE_DEFAULT, null), + 'type' => new external_value(PARAM_INT, 'The type of the conversation (1=individual,2=group)'), + 'membercount' => new external_value(PARAM_INT, 'Total number of conversation members'), + 'isfavourite' => new external_value(PARAM_BOOL, 'If the user marked conversation this conversation as a favourite'), + 'isread' => new external_value(PARAM_BOOL, 'If the user has read all messages in the conversation'), + 'unreadcount' => new external_value(PARAM_INT, 'The number of unread messages in this conversation', + VALUE_DEFAULT, null), + 'members' => new external_multiple_structure( + self::get_conversation_member_structure() + ), + 'messages' => new external_multiple_structure( + self::get_conversation_message_structure() + ), + ) + ); + } + /** * Return the structure of a conversation member. * @@ -1165,6 +1193,83 @@ class core_message_external extends external_api { ); } + /** + * Get conversations parameters. + * + * @return external_function_parameters + * @since 3.6 + */ + public static function get_conversations_parameters() { + return new external_function_parameters( + array( + 'userid' => new external_value(PARAM_INT, 'The id of the user who we are viewing conversations for'), + 'limitfrom' => new external_value(PARAM_INT, 'The offset to start at', VALUE_DEFAULT, 0), + 'limitnum' => new external_value(PARAM_INT, 'Limit number of conversations to this', VALUE_DEFAULT, 0), + 'type' => new external_value(PARAM_INT, 'Filter by type', VALUE_DEFAULT, null), + 'favourites' => new external_value(PARAM_BOOL, 'Whether to restrict the results to contain NO favourite + conversations (false), ONLY favourite conversation (true), or ignore any restriction altogether (null)', + VALUE_DEFAULT, null), + + ) + ); + } + + /** + * Get the list of conversations for the user. + * + * @param int $userid The id of the user who is performing the search + * @param int $limitfrom + * @param int $limitnum + * @param int|null $type + * @param bool|null $favourites + * @return stdClass + * @throws \moodle_exception if the messaging feature is disabled on the site. + * @since 3.2 + */ + public static function get_conversations($userid, $limitfrom = 0, $limitnum = 0, int $type = null, bool $favourites = null) { + global $CFG, $USER; + + // All the standard BL checks. + if (empty($CFG->messaging)) { + throw new moodle_exception('disabled', 'message'); + } + + $params = array( + 'userid' => $userid, + 'limitfrom' => $limitfrom, + 'limitnum' => $limitnum, + 'type' => $type, + 'favourites' => $favourites + ); + self::validate_parameters(self::get_conversations_parameters(), $params); + + $systemcontext = context_system::instance(); + self::validate_context($systemcontext); + + if (($USER->id != $userid) && !has_capability('moodle/site:readallmessages', $systemcontext)) { + throw new moodle_exception('You do not have permission to perform this action.'); + } + + $conversations = \core_message\api::get_conversations($userid, $limitfrom, $limitnum, $type, $favourites); + return (object) ['conversations' => $conversations]; + } + + /** + * Get conversations returns. + * + * @return external_single_structure + * @since 3.6 + */ + public static function get_conversations_returns() { + return new external_single_structure( + [ + 'conversations' => new external_multiple_structure( + self::get_conversation_structure() + ) + ] + ); + } + /** * The messagearea conversations parameters. * diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index f406dbdbcb9..11cea2187ff 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -4339,4 +4339,343 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->expectException(\moodle_exception::class); $result = core_message_external::unset_favourite_conversations($user1->id, [0]); } + + /** + * Helper to seed the database with initial state. + */ + protected function create_conversation_test_data() { + // Create some users. + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + $user4 = self::getDataGenerator()->create_user(); + + $time = 1; + + // Create some conversations. We want: + // 1) At least one of each type (group, individual) of which user1 IS a member and DID send the most recent message. + // 2) At least one of each type (group, individual) of which user1 IS a member and DID NOT send the most recent message. + // 3) At least one of each type (group, individual) of which user1 IS NOT a member. + // 4) At least two group conversation having 0 messages, of which user1 IS a member (To confirm conversationid ordering). + // 5) At least one group conversation having 0 messages, of which user1 IS NOT a member. + + // Individual conversation, user1 is a member, last message from other user. + $ic1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$user1->id, $user2->id]); + testhelper::send_fake_message_to_conversation($user1, $ic1->id, 'Message 1', $time); + testhelper::send_fake_message_to_conversation($user2, $ic1->id, 'Message 2', $time + 1); + + // Individual conversation, user1 is a member, last message from user1. + $ic2 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$user1->id, $user3->id]); + testhelper::send_fake_message_to_conversation($user3, $ic2->id, 'Message 3', $time + 2); + testhelper::send_fake_message_to_conversation($user1, $ic2->id, 'Message 4', $time + 3); + + // Individual conversation, user1 is not a member. + $ic3 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$user2->id, $user3->id]); + testhelper::send_fake_message_to_conversation($user2, $ic3->id, 'Message 5', $time + 4); + testhelper::send_fake_message_to_conversation($user3, $ic3->id, 'Message 6', $time + 5); + + // Group conversation, user1 is not a member. + $gc1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user2->id, $user3->id, $user4->id], 'Project discussions'); + testhelper::send_fake_message_to_conversation($user2, $gc1->id, 'Message 7', $time + 6); + testhelper::send_fake_message_to_conversation($user4, $gc1->id, 'Message 8', $time + 7); + + // Group conversation, user1 is a member, last message from another user. + $gc2 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user1->id, $user3->id, $user4->id], 'Group chat'); + testhelper::send_fake_message_to_conversation($user1, $gc2->id, 'Message 9', $time + 8); + testhelper::send_fake_message_to_conversation($user3, $gc2->id, 'Message 10', $time + 9); + testhelper::send_fake_message_to_conversation($user4, $gc2->id, 'Message 11', $time + 10); + + // Group conversation, user1 is a member, last message from user1. + $gc3 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user1->id, $user2->id, $user3->id, $user4->id], 'Group chat again!'); + testhelper::send_fake_message_to_conversation($user4, $gc3->id, 'Message 12', $time + 11); + testhelper::send_fake_message_to_conversation($user3, $gc3->id, 'Message 13', $time + 12); + testhelper::send_fake_message_to_conversation($user1, $gc3->id, 'Message 14', $time + 13); + + // Empty group conversations (x2), user1 is a member. + $gc4 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user1->id, $user2->id, $user3->id], 'Empty group'); + $gc5 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user1->id, $user2->id, $user4->id], 'Another empty group'); + + // Empty group conversation, user1 is NOT a member. + $gc6 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$user2->id, $user3->id, $user4->id], 'Empty group 3'); + + return [$user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, $gc1, $gc2, $gc3, $gc4, $gc5, $gc6]; + } + + /** + * Test confirming the basic use of get_conversations, with no limits, nor type or favourite restrictions. + */ + public function test_get_conversations_no_restrictions() { + $this->resetAfterTest(true); + + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // The user making the request. + $this->setUser($user1); + + // Get all conversations for user1. + $result = core_message_external::get_conversations($user1->id); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + + // Verify there are 6 conversations: 2 individual, 2 group with message, and 2 group without messages. + // The conversations with the most recent messages should be listed first, followed by the most newly created + // conversations without messages. + $this->assertCount(6, $conversations); + $this->assertEquals($gc3->id, $conversations[0]['id']); + $this->assertEquals($gc2->id, $conversations[1]['id']); + $this->assertEquals($ic2->id, $conversations[2]['id']); + $this->assertEquals($ic1->id, $conversations[3]['id']); + $this->assertEquals($gc5->id, $conversations[4]['id']); + $this->assertEquals($gc4->id, $conversations[5]['id']); + + foreach ($conversations as $conv) { + $this->assertArrayHasKey('id', $conv); + $this->assertArrayHasKey('name', $conv); + $this->assertArrayHasKey('subname', $conv); + $this->assertArrayHasKey('type', $conv); + $this->assertArrayHasKey('membercount', $conv); + $this->assertArrayHasKey('isfavourite', $conv); + $this->assertArrayHasKey('isread', $conv); + $this->assertArrayHasKey('unreadcount', $conv); + $this->assertArrayHasKey('members', $conv); + foreach ($conv['members'] as $member) { + $this->assertArrayHasKey('id', $member); + $this->assertArrayHasKey('fullname', $member); + $this->assertArrayHasKey('profileimageurl', $member); + $this->assertArrayHasKey('profileimageurlsmall', $member); + $this->assertArrayHasKey('isonline', $member); + $this->assertArrayHasKey('showonlinestatus', $member); + $this->assertArrayHasKey('isblocked', $member); + $this->assertArrayHasKey('iscontact', $member); + } + $this->assertArrayHasKey('messages', $conv); + foreach ($conv['messages'] as $message) { + $this->assertArrayHasKey('id', $message); + $this->assertArrayHasKey('useridfrom', $message); + $this->assertArrayHasKey('text', $message); + $this->assertArrayHasKey('timecreated', $message); + } + } + } + + /** + * Tests retrieving conversations with a limit and offset to ensure pagination works correctly. + */ + public function test_get_conversations_limit_offset() { + $this->resetAfterTest(true); + + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // The user making the request. + $this->setUser($user1); + + // Get all conversations for user1. + $result = core_message_external::get_conversations($user1->id, 0, 1); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + + // Verify the first conversation. + $this->assertCount(1, $conversations); + $conversation = array_shift($conversations); + $this->assertEquals($gc3->id, $conversation['id']); + + // Verify the next conversation. + $result = core_message_external::get_conversations($user1->id, 1, 1); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(1, $conversations); + $this->assertEquals($gc2->id, $conversations[0]['id']); + + // Verify the next conversation. + $result = core_message_external::get_conversations($user1->id, 2, 1); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(1, $conversations); + $this->assertEquals($ic2->id, $conversations[0]['id']); + + // Skip one and get both empty conversations. + $result = core_message_external::get_conversations($user1->id, 4, 2); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(2, $conversations); + $this->assertEquals($gc5->id, $conversations[0]['id']); + $this->assertEmpty($conversations[0]['messages']); + $this->assertEquals($gc4->id, $conversations[1]['id']); + $this->assertEmpty($conversations[1]['messages']); + + // Ask for an offset that doesn't exist and verify no conversations are returned. + $conversations = \core_message\api::get_conversations($user1->id, 10, 1); + $this->assertCount(0, $conversations); + } + + /** + * Test verifying the type filtering behaviour of the get_conversations external method. + */ + public function test_get_conversations_type_filter() { + $this->resetAfterTest(true); + + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // The user making the request. + $this->setUser($user1); + + // Verify we can ask for only individual conversations. + $result = core_message_external::get_conversations($user1->id, 0, 20, + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(2, $conversations); + + // Verify we can ask for only group conversations. + $result = core_message_external::get_conversations($user1->id, 0, 20, + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(4, $conversations); + + // Verify an exception is thrown if an unrecognized type is specified. + $this->expectException(\moodle_exception::class); + core_message_external::get_conversations($user1->id, 0, 20, 0); + } + + /** + * Tests retrieving conversations when a conversation contains a deleted user. + */ + public function test_get_conversations_deleted_user() { + $this->resetAfterTest(true); + + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // The user making the request. + $this->setUser($user1); + + // Delete the second user and retrieve the conversations. + // We should have 5, as $ic1 drops off the list. + // Group conversations remain albeit with less members. + delete_user($user2); + $result = core_message_external::get_conversations($user1->id); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(5, $conversations); + $this->assertEquals($gc3->id, $conversations[0]['id']); + $this->assertcount(1, $conversations[0]['members']); + $this->assertEquals($gc2->id, $conversations[1]['id']); + $this->assertcount(1, $conversations[1]['members']); + $this->assertEquals($ic2->id, $conversations[2]['id']); + $this->assertEquals($gc5->id, $conversations[3]['id']); + $this->assertEquals($gc4->id, $conversations[4]['id']); + + // Delete a user from a group conversation where that user had sent the most recent message. + // This user will still be present in the members array, as will the message in the messages array. + delete_user($user4); + $result = core_message_external::get_conversations($user1->id); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(5, $conversations); + $this->assertEquals($gc2->id, $conversations[1]['id']); + $this->assertcount(1, $conversations[1]['members']); + $this->assertEquals($user4->id, $conversations[1]['members'][0]['id']); + $this->assertcount(1, $conversations[1]['messages']); + $this->assertEquals($user4->id, $conversations[1]['messages'][0]['useridfrom']); + + // Delete the third user and retrieve the conversations. + // We should have 4, as $ic1, $ic2 drop off the list. + // Group conversations remain albeit with less members. + delete_user($user3); + $result = core_message_external::get_conversations($user1->id); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(4, $conversations); + $this->assertEquals($gc3->id, $conversations[0]['id']); + $this->assertcount(1, $conversations[0]['members']); + $this->assertEquals($gc2->id, $conversations[1]['id']); + $this->assertcount(1, $conversations[1]['members']); + $this->assertEquals($gc5->id, $conversations[2]['id']); + $this->assertEquals($gc4->id, $conversations[3]['id']); + } + + /** + * Test verifying the behaviour of get_conversations() when fetching favourite conversations. + */ + public function test_get_conversations_favourite_conversations() { + $this->resetAfterTest(true); + + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // The user making the request. + $this->setUser($user1); + + // Try to get ONLY favourite conversations, when no favourites exist. + $result = core_message_external::get_conversations($user1->id, 0, 20, null, true); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertEquals([], $conversations); + + // Try to get NO favourite conversations, when no favourites exist. + $result = core_message_external::get_conversations($user1->id, 0, 20, null, false); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(6, $conversations); + + // Mark a few conversations as favourites. + \core_message\api::set_favourite_conversation($ic1->id, $user1->id); + \core_message\api::set_favourite_conversation($gc2->id, $user1->id); + \core_message\api::set_favourite_conversation($gc5->id, $user1->id); + + // Get the conversations, first with no restrictions, confirming the favourite status of the conversations. + $result = core_message_external::get_conversations($user1->id); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(6, $conversations); + foreach ($conversations as $conv) { + if (in_array($conv['id'], [$ic1->id, $gc2->id, $gc5->id])) { + $this->assertTrue($conv['isfavourite']); + } + } + + // Now, get ONLY favourite conversations. + $result = core_message_external::get_conversations($user1->id, 0, 20, null, true); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(3, $conversations); + foreach ($conversations as $conv) { + $this->assertTrue($conv['isfavourite']); + } + + // Now, try ONLY favourites of type 'group'. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, true); + $this->assertCount(2, $conversations); + foreach ($conversations as $conv) { + $this->assertTrue($conv->isfavourite); + } + + // And NO favourite conversations. + $result = core_message_external::get_conversations($user1->id, 0, 20, null, false); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + $this->assertCount(3, $conversations); + foreach ($conversations as $conv) { + $this->assertFalse($conv['isfavourite']); + } + } } From eb35e0b176634675a62e8858781661a879ee7a1e Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Tue, 23 Oct 2018 19:08:24 +0800 Subject: [PATCH 3/3] MDL-63549 core_message: deprecate data_for_messagearea_conversations --- lib/db/services.php | 3 ++- message/classes/helper.php | 33 +++++++++++++++++++++++++++++++++ message/externallib.php | 22 ++++++++++++++++++++++ message/index.php | 3 +++ message/upgrade.txt | 2 ++ 5 files changed, 62 insertions(+), 1 deletion(-) diff --git a/lib/db/services.php b/lib/db/services.php index a9dcdd3d73d..be6c06c5a24 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -981,7 +981,8 @@ $functions = array( 'classname' => 'core_message_external', 'methodname' => 'data_for_messagearea_conversations', 'classpath' => 'message/externallib.php', - 'description' => 'Retrieve the template data for the conversation list', + 'description' => '** DEPRECATED ** Please do not call this function any more. + Retrieve the template data for the conversation list', 'type' => 'read', 'ajax' => true, 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), diff --git a/message/classes/helper.php b/message/classes/helper.php index a1ce9eb4bf1..67dcf871ca5 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -529,4 +529,37 @@ class helper { } return $members; } + + /** + * Backwards compatibility formatter, transforming the new output of get_conversations() into the old format. + * + * @param array $conversations the array of conversations, which must come from get_conversations(). + * @return array the array of conversations, formatted in the legacy style. + */ + public static function get_conversations_legacy_formatter(array $conversations) : array { + // Transform new data format back into the old format, just for BC during the deprecation life cycle. + $tmp = []; + foreach ($conversations as $id => $conv) { + $data = new \stdClass(); + // The logic for the 'other user' is as follows: + // If a conversation is of type 'individual', the other user is always the member who is not the current user. + // If the conversation is of type 'group', the other user is always the sender of the most recent message. + // The get_conversations method already follows this logic, so we just need the first member. + $otheruser = reset($conv->members); + $data->userid = $otheruser->id; + $data->useridfrom = $conv->messages[0]->useridfrom ?? null; + $data->fullname = $conv->members[$otheruser->id]->fullname; + $data->profileimageurl = $conv->members[$otheruser->id]->profileimageurl; + $data->profileimageurlsmall = $conv->members[$otheruser->id]->profileimageurlsmall; + $data->ismessaging = isset($conv->messages[0]->text) ? true : false; + $data->lastmessage = $conv->messages[0]->text ?? null; + $data->messageid = $conv->messages[0]->id ?? null; + $data->isonline = $conv->members[$otheruser->id]->isonline ?? null; + $data->isblocked = $conv->members[$otheruser->id]->isblocked ?? null; + $data->isread = $conv->isread; + $data->unreadcount = $conv->unreadcount; + $tmp[$data->userid] = $data; + } + return $tmp; + } } diff --git a/message/externallib.php b/message/externallib.php index ec7fa2ce2f3..0d71f3fadbc 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -1273,6 +1273,7 @@ class core_message_external extends external_api { /** * The messagearea conversations parameters. * + * @deprecated since 3.6 * @return external_function_parameters * @since 3.2 */ @@ -1289,6 +1290,13 @@ class core_message_external extends external_api { /** * Get messagearea conversations. * + * NOTE FOR FINAL DEPRECATION: + * When removing this method, please also consider removal of get_conversations_legacy_formatter() + * from the \core_message\helper class. This helper method was used solely to format the new get_conversations() return data + * into the old format used here, and in message/index.php. If we no longer need either of these, then that method can be + * removed. + * + * @deprecated since 3.6 * @param int $userid The id of the user who we are viewing conversations for * @param int $limitfrom * @param int $limitnum @@ -1319,6 +1327,10 @@ class core_message_external extends external_api { } $conversations = \core_message\api::get_conversations($userid, $limitfrom, $limitnum); + + // Format the conversations in the legacy style, as the get_conversations method has since been changed. + $conversations = \core_message\helper::get_conversations_legacy_formatter($conversations); + $conversations = new \core_message\output\messagearea\contacts(null, $conversations); $renderer = $PAGE->get_renderer('core_message'); @@ -1328,6 +1340,7 @@ class core_message_external extends external_api { /** * The messagearea conversations return structure. * + * @deprecated since 3.6 * @return external_single_structure * @since 3.2 */ @@ -1341,6 +1354,15 @@ class core_message_external extends external_api { ); } + /** + * Marking the method as deprecated. + * + * @return bool + */ + public static function data_for_messagearea_conversations_is_deprecated() { + return true; + } + /** * The messagearea contacts return parameters. * diff --git a/message/index.php b/message/index.php index 45cf6058d4e..9d30f34e12b 100644 --- a/message/index.php +++ b/message/index.php @@ -106,6 +106,9 @@ if ($contactsfirst) { $conversations = \core_message\api::get_contacts($user1->id, 0, 20); } else { $conversations = \core_message\api::get_conversations($user1->id, 0, 20); + + // Format the conversations in the legacy style, as the get_conversations method has since been changed. + $conversations = \core_message\helper::get_conversations_legacy_formatter($conversations); } $messages = []; if (!$user2realuser) { diff --git a/message/upgrade.txt b/message/upgrade.txt index a74dc189fb8..5eaf7926325 100644 --- a/message/upgrade.txt +++ b/message/upgrade.txt @@ -44,6 +44,8 @@ information provided here is intended especially for developers. - core_message_external::delete_conversation(), please use core_message_external::delete_conversations_by_id() instead. - core_message_external::core_message_mark_all_messages_as_read(), please use core_message_external::core_message_mark_all_conversation_messages_as_read() instead. + - core_message_external::data_for_messagearea_conversations(), please use core_message_external::get_conversations() + instead * The following function has been added for getting the privacy messaging preference: - get_user_privacy_messaging_preference()