diff --git a/message/classes/api.php b/message/classes/api.php index 291e0127e3b..abc917c22f5 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -218,23 +218,22 @@ class api { $excludeusers = array($userid, $CFG->siteguest); list($exclude, $excludeparams) = $DB->get_in_or_equal($excludeusers, SQL_PARAMS_NAMED, 'param', false); - $params = array('search' => '%' . $search . '%', 'userid1' => $userid, 'userid2' => $userid, 'userid3' => $userid); - // Ok, let's search for contacts first. $contacts = array(); $sql = "SELECT $ufields, mub.id as isuserblocked - FROM {user} u - JOIN {message_contacts} mc - ON (u.id = mc.contactid AND mc.userid = :userid1) OR (u.id = mc.userid AND mc.contactid = :userid2) - LEFT JOIN {message_users_blocked} mub - ON (mub.userid = :userid3 AND mub.blockeduserid = u.id) - WHERE u.deleted = 0 - AND u.confirmed = 1 - AND " . $DB->sql_like($fullname, ':search', false) . " - AND u.id $exclude - ORDER BY " . $DB->sql_fullname(); - - if ($users = $DB->get_records_sql($sql, $params + $excludeparams, 0, $limitnum)) { + FROM {user} u + JOIN {message_contacts} mc + ON u.id = mc.contactid + LEFT JOIN {message_users_blocked} mub + ON (mub.userid = :userid2 AND mub.blockeduserid = u.id) + WHERE mc.userid = :userid + AND u.deleted = 0 + AND u.confirmed = 1 + AND " . $DB->sql_like($fullname, ':search', false) . " + AND u.id $exclude + ORDER BY " . $DB->sql_fullname(); + if ($users = $DB->get_records_sql($sql, array('userid' => $userid, 'userid2' => $userid, + 'search' => '%' . $search . '%') + $excludeparams, 0, $limitnum)) { foreach ($users as $user) { $user->blocked = $user->isuserblocked ? 1 : 0; $contacts[] = helper::create_contact($user); @@ -262,46 +261,22 @@ class api { } } - // Let's get those non-contacts. + // Let's get those non-contacts. Toast them gears boi. + // Note - you can only block contacts, so these users will not be blocked, so no need to get that + // extra detail from the database. $noncontacts = array(); - if ($CFG->messagingallusers) { - // In case $CFG->messagingallusers is enabled, search for all users site-wide but are not user's contact. - $sql = "SELECT $ufields - FROM {user} u - LEFT JOIN {message_users_blocked} mub - ON (mub.userid = :userid1 AND mub.blockeduserid = u.id) - WHERE u.deleted = 0 - AND u.confirmed = 1 - AND " . $DB->sql_like($fullname, ':search', false) . " - AND u.id $exclude - AND NOT EXISTS (SELECT mc.id - FROM {message_contacts} mc - WHERE (mc.userid = u.id AND mc.contactid = :userid2) - OR (mc.userid = :userid3 AND mc.contactid = u.id)) - ORDER BY " . $DB->sql_fullname(); - } else { - // In case $CFG->messagingallusers is disabled, search for users you have a conversation with. - // Messaging setting could change, so could exist an old conversation with users you cannot message anymore. - $sql = "SELECT $ufields, mub.id as isuserblocked - FROM {user} u - LEFT JOIN {message_users_blocked} mub - ON (mub.userid = :userid1 AND mub.blockeduserid = u.id) - INNER JOIN {message_conversation_members} cm - ON u.id = cm.userid - INNER JOIN {message_conversation_members} cm2 - ON cm.conversationid = cm2.conversationid AND cm2.userid = :userid - WHERE u.deleted = 0 - AND u.confirmed = 1 - AND " . $DB->sql_like($fullname, ':search', false) . " - AND u.id $exclude - AND NOT EXISTS (SELECT mc.id - FROM {message_contacts} mc - WHERE (mc.userid = u.id AND mc.contactid = :userid2) - OR (mc.userid = :userid3 AND mc.contactid = u.id)) - ORDER BY " . $DB->sql_fullname(); - $params['userid'] = $userid; - } - if ($users = $DB->get_records_sql($sql, $params + $excludeparams, 0, $limitnum)) { + $sql = "SELECT $ufields + FROM {user} u + WHERE u.deleted = 0 + AND u.confirmed = 1 + AND " . $DB->sql_like($fullname, ':search', false) . " + AND u.id $exclude + AND u.id NOT IN (SELECT contactid + FROM {message_contacts} + WHERE userid = :userid) + ORDER BY " . $DB->sql_fullname(); + if ($users = $DB->get_records_sql($sql, array('userid' => $userid, 'search' => '%' . $search . '%') + $excludeparams, + 0, $limitnum)) { foreach ($users as $user) { $noncontacts[] = helper::create_contact($user); } @@ -319,9 +294,14 @@ class api { * @param int $limitnum * @return array */ - public static function message_search_users(int $userid, string $search, int $limitfrom = 0, int $limitnum = 1000) : array { + public static function message_search_users(int $userid, string $search, int $limitfrom = 0, int $limitnum = 20) : array { global $CFG, $DB; + // Check if messaging is enabled. + if (empty($CFG->messaging)) { + throw new \moodle_exception('disabled', 'message'); + } + // Used to search for contacts. $fullname = $DB->sql_fullname(); @@ -343,7 +323,7 @@ class api { ORDER BY " . $DB->sql_fullname(); $foundusers = $DB->get_records_sql_menu($sql, $params + $excludeparams, $limitfrom, $limitnum); - $orderedcontacs = array(); + $orderedcontacts = array(); if (!empty($foundusers)) { $contacts = helper::get_member_info($userid, array_keys($foundusers)); // The get_member_info returns an associative array, so is not ordered in the same way. @@ -351,47 +331,93 @@ class api { foreach ($foundusers as $key => $value) { $contact = $contacts[$key]; $contact->conversations = self::get_conversations_between_users($userid, $key, 0, 1000); - $orderedcontacs[] = $contact; + $orderedcontacts[] = $contact; } } // Let's get those non-contacts. + // If site wide messaging is enabled, we just fetch any matched users which are non-contacts. if ($CFG->messagingallusers) { - // In case $CFG->messagingallusers is enabled, search for all users site-wide but are not user's contact. $sql = "SELECT u.id - FROM {user} u - WHERE u.deleted = 0 - AND u.confirmed = 1 - AND " . $DB->sql_like($fullname, ':search', false) . " - AND u.id $exclude - AND NOT EXISTS (SELECT mc.id - FROM {message_contacts} mc - WHERE (mc.userid = u.id AND mc.contactid = :userid1) - OR (mc.userid = :userid2 AND mc.contactid = u.id)) - ORDER BY " . $DB->sql_fullname(); - } else { - // In case $CFG->messagingallusers is disabled, search for users you have a conversation with. - // Messaging setting could change, so could exist an old conversation with users you cannot message anymore. - $sql = "SELECT u.id - FROM {user} u - INNER JOIN {message_conversation_members} cm - ON u.id = cm.userid - INNER JOIN {message_conversation_members} cm2 - ON cm.conversationid = cm2.conversationid AND cm2.userid = :userid - WHERE u.deleted = 0 - AND u.confirmed = 1 - AND " . $DB->sql_like($fullname, ':search', false) . " - AND u.id $exclude - AND NOT EXISTS (SELECT mc.id - FROM {message_contacts} mc - WHERE (mc.userid = u.id AND mc.contactid = :userid1) - OR (mc.userid = :userid2 AND mc.contactid = u.id)) - ORDER BY " . $DB->sql_fullname(); - $params['userid'] = $userid; - } - $foundusers = $DB->get_records_sql_menu($sql, $params + $excludeparams, $limitfrom, $limitnum); + FROM {user} u + WHERE u.deleted = 0 + AND u.confirmed = 1 + AND " . $DB->sql_like($fullname, ':search', false) . " + AND u.id $exclude + AND NOT EXISTS (SELECT mc.id + FROM {message_contacts} mc + WHERE (mc.userid = u.id AND mc.contactid = :userid1) + OR (mc.userid = :userid2 AND mc.contactid = u.id)) + ORDER BY " . $DB->sql_fullname(); - $orderednoncontacs = array(); + $foundusers = $DB->get_records_sql($sql, $params + $excludeparams, $limitfrom, $limitnum); + } else { + require_once($CFG->dirroot . '/user/lib.php'); + // If site-wide messaging is disabled, then we should only be able to search for users who we are allowed to see. + // Because we can't achieve all the required visibility checks in SQL, we'll iterate through the non-contact records + // and stop once we have enough matching the 'visible' criteria. + // TODO: MDL-63983 - Improve the performance of non-contact searches when site-wide messaging is disabled (default). + + // Use a local generator to achieve this iteration. + $getnoncontactusers = function ($limitfrom = 0, $limitnum = 0) use($fullname, $exclude, $params, $excludeparams) { + global $DB; + $sql = "SELECT u.* + FROM {user} u + WHERE u.deleted = 0 + AND u.confirmed = 1 + AND " . $DB->sql_like($fullname, ':search', false) . " + AND u.id $exclude + AND NOT EXISTS (SELECT mc.id + FROM {message_contacts} mc + WHERE (mc.userid = u.id AND mc.contactid = :userid1) + OR (mc.userid = :userid2 AND mc.contactid = u.id)) + ORDER BY " . $DB->sql_fullname(); + while ($records = $DB->get_records_sql($sql, $params + $excludeparams, $limitfrom, $limitnum)) { + yield $records; + $limitfrom += $limitnum; + } + }; + + // Fetch in batches of $limitnum * 2 to improve the chances of matching a user without going back to the DB. + // The generator cannot function without a sensible limiter, so set one if this is not set. + $batchlimit = ($limitnum == 0) ? 20 : $limitnum; + + // We need to make the offset param work with the generator. + // Basically, if we want to get say 10 records starting at the 40th record, we need to see 50 records and return only + // those after the 40th record. We can never pass the method's offset param to the generator as we need to manage the + // position within those valid records ourselves. + // See MDL-63983 dealing with performance improvements to this area of code. + $noofvalidseenrecords = 0; + $returnedusers = []; + foreach ($getnoncontactusers(0, $batchlimit) as $users) { + foreach ($users as $id => $user) { + $userdetails = \user_get_user_details_courses($user); + + // Return the user only if the searched field is returned. + // Otherwise it means that the $USER was not allowed to search the returned user. + if (!empty($userdetails) and !empty($userdetails['fullname'])) { + // We know we've matched, but only save the record if it's within the offset area we need. + if ($limitfrom == 0) { + // No offset specified, so just save. + $returnedusers[$id] = $user; + } else { + // There is an offset in play. + // If we've passed enough records already (> offset value), then we can save this one. + if ($noofvalidseenrecords >= $limitfrom) { + $returnedusers[$id] = $user; + } + } + if (count($returnedusers) == $limitnum) { + break 2; + } + $noofvalidseenrecords++; + } + } + } + $foundusers = $returnedusers; + } + + $orderednoncontacts = array(); if (!empty($foundusers)) { $noncontacts = helper::get_member_info($userid, array_keys($foundusers)); // The get_member_info returns an associative array, so is not ordered in the same way. @@ -399,11 +425,11 @@ class api { foreach ($foundusers as $key => $value) { $contact = $noncontacts[$key]; $contact->conversations = self::get_conversations_between_users($userid, $key, 0, 1000); - $orderednoncontacs[] = $contact; + $orderednoncontacts[] = $contact; } } - return array($orderedcontacs, $orderednoncontacs); + return array($orderedcontacts, $orderednoncontacts); } /** diff --git a/message/externallib.php b/message/externallib.php index d2007782b6c..0c9c0559007 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -1366,12 +1366,7 @@ class core_message_external extends external_api { * @since 3.6 */ public static function message_search_users($userid, $search, $limitfrom = 0, $limitnum = 0) { - global $CFG, $USER; - - // Check if messaging is enabled. - if (empty($CFG->messaging)) { - throw new moodle_exception('disabled', 'message'); - } + global $USER; $systemcontext = context_system::instance(); diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 145ec68cd7d..78dfbfd2f53 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -307,87 +307,346 @@ class core_message_api_testcase extends core_message_messagelib_testcase { } /** - * Tests searching users. + * Tests searching for users when site-wide messaging is disabled. + * + * This test verifies that any contacts are returned, as well as any non-contacts whose profile we can view. + * If checks this by placing some users in the same course, where default caps would permit a user to view another user's + * profile. */ - public function test_message_search_users() { + public function test_message_search_users_messagingallusers_disabled() { + $this->resetAfterTest(); + // Create some users. - $user1 = new stdClass(); - $user1->firstname = 'User search'; - $user1->lastname = 'One'; - $user1 = self::getDataGenerator()->create_user($user1); + $users = []; + foreach (range(1, 7) as $i) { + $user = new stdClass(); + $user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term. + $user->lastname = $i; + $user = $this->getDataGenerator()->create_user($user); + $users[$i] = $user; + } + + // Enrol a few users in the same course, but leave them as non-contacts. + $course1 = $this->getDataGenerator()->create_course(); + $this->setAdminUser(); + $this->getDataGenerator()->enrol_user($users[1]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[6]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[7]->id, $course1->id); + + // Add some other users as contacts. + \core_message\api::add_contact($users[1]->id, $users[2]->id); + \core_message\api::add_contact($users[3]->id, $users[1]->id); + \core_message\api::add_contact($users[1]->id, $users[4]->id); + + // Create individual conversations between some users, one contact and one non-contact. + $ic1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$users[1]->id, $users[2]->id]); + $ic2 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$users[6]->id, $users[1]->id]); + + // Create a group conversation between 4 users, including a contact and a non-contact. + $gc1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$users[1]->id, $users[2]->id, $users[4]->id, $users[7]->id], 'Project chat'); // Set as the user performing the search. - $this->setUser($user1); + $this->setUser($users[1]); - $user2 = new stdClass(); - $user2->firstname = 'User search'; - $user2->lastname = 'Two'; - $user2 = self::getDataGenerator()->create_user($user2); + // Perform a search with $CFG->messagingallusers disabled. + set_config('messagingallusers', 0); + $result = \core_message\api::message_search_users($users[1]->id, 'search'); - $user3 = new stdClass(); - $user3->firstname = 'User search'; - $user3->lastname = 'Three'; - $user3 = self::getDataGenerator()->create_user($user3); - - $user4 = new stdClass(); - $user4->firstname = 'User'; - $user4->lastname = 'Four'; - $user4 = self::getDataGenerator()->create_user($user4); - - $user5 = new stdClass(); - $user5->firstname = 'User search'; - $user5->lastname = 'Five'; - $user5 = self::getDataGenerator()->create_user($user5); - - $user6 = new stdClass(); - $user6->firstname = 'User search'; - $user6->lastname = 'Six'; - $user6 = self::getDataGenerator()->create_user($user6); - - $user7 = new stdClass(); - $user7->firstname = 'User search'; - $user7->lastname = 'Seven'; - $user7 = self::getDataGenerator()->create_user($user7); - - // Add some users as contacts. - \core_message\api::add_contact($user1->id, $user2->id); - \core_message\api::add_contact($user3->id, $user1->id); - \core_message\api::add_contact($user1->id, $user4->id); - - // Create private conversations with some users. - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user1->id, $user6->id)); - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user7->id, $user1->id)); - - // Perform a search $CFG->messagingallusers setting enabled. - set_config('messagingallusers', 1); - list($contacts, $noncontacts) = \core_message\api::message_search_users($user1->id, 'search'); + // Confirm that we returns contacts and non-contacts. + $this->assertArrayHasKey(0, $result); + $this->assertArrayHasKey(1, $result); + $contacts = $result[0]; + $noncontacts = $result[1]; // Check that we retrieved the correct contacts. $this->assertCount(2, $contacts); - $this->assertEquals($user3->id, $contacts[0]->id); - $this->assertEquals($user2->id, $contacts[1]->id); + $this->assertEquals($users[2]->id, $contacts[0]->id); + $this->assertEquals($users[3]->id, $contacts[1]->id); + + // Verify the correct conversations were returned for the contacts. + $this->assertCount(2, $contacts[0]->conversations); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $contacts[0]->conversations[$gc1->id]->type); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $contacts[0]->conversations[$ic1->id]->type); + + $this->assertCount(0, $contacts[1]->conversations); + + // Check that we retrieved the correct non-contacts. + // When site wide messaging is disabled, we expect to see only those users whose profiles we can view. + $this->assertCount(2, $noncontacts); + $this->assertEquals($users[6]->id, $noncontacts[0]->id); + $this->assertEquals($users[7]->id, $noncontacts[1]->id); + + // Verify the correct conversations were returned for the non-contacts. + $this->assertCount(1, $noncontacts[0]->conversations); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + $noncontacts[0]->conversations[$ic2->id]->type); + + $this->assertCount(1, $noncontacts[1]->conversations); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[1]->conversations[$gc1->id]->type); + } + + /** + * Tests searching for users when site-wide messaging is enabled. + * + * This test verifies that any contacts are returned, as well as any non-contacts, regardless of whether the searching user + * can view their respective profile. + */ + public function test_message_search_users_messagingallusers_enabled() { + $this->resetAfterTest(); + + // Create some users. + $users = []; + foreach (range(1, 8) as $i) { + $user = new stdClass(); + $user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term. + $user->lastname = $i; + $user = $this->getDataGenerator()->create_user($user); + $users[$i] = $user; + } + + // Enrol a few users in the same course, but leave them as non-contacts. + $course1 = $this->getDataGenerator()->create_course(); + $this->setAdminUser(); + $this->getDataGenerator()->enrol_user($users[1]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[6]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[7]->id, $course1->id); + + // Add some other users as contacts. + \core_message\api::add_contact($users[1]->id, $users[2]->id); + \core_message\api::add_contact($users[3]->id, $users[1]->id); + \core_message\api::add_contact($users[1]->id, $users[4]->id); + + // Create individual conversations between some users, one contact and one non-contact. + $ic1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$users[1]->id, $users[2]->id]); + $ic2 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$users[6]->id, $users[1]->id]); + + // Create a group conversation between 5 users, including a contact and a non-contact, and a user NOT in a shared course. + $gc1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$users[1]->id, $users[2]->id, $users[4]->id, $users[7]->id, $users[8]->id], 'Project chat'); + + // Set as the user performing the search. + $this->setUser($users[1]); + + // Perform a search with $CFG->messagingallusers enabled. + set_config('messagingallusers', 1); + $result = \core_message\api::message_search_users($users[1]->id, 'search'); + + // Confirm that we returns contacts and non-contacts. + $this->assertArrayHasKey(0, $result); + $this->assertArrayHasKey(1, $result); + $contacts = $result[0]; + $noncontacts = $result[1]; + + // Check that we retrieved the correct contacts. + $this->assertCount(2, $contacts); + $this->assertEquals($users[2]->id, $contacts[0]->id); + $this->assertEquals($users[3]->id, $contacts[1]->id); + + // Verify the correct conversations were returned for the contacts. + $this->assertCount(2, $contacts[0]->conversations); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $contacts[0]->conversations[$gc1->id]->type); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $contacts[0]->conversations[$ic1->id]->type); + + $this->assertCount(0, $contacts[1]->conversations); + + // Check that we retrieved the correct non-contacts. + // If site wide messaging is enabled, we expect to be able to search for any users. + $this->assertCount(4, $noncontacts); + $this->assertEquals($users[5]->id, $noncontacts[0]->id); + $this->assertEquals($users[6]->id, $noncontacts[1]->id); + $this->assertEquals($users[7]->id, $noncontacts[2]->id); + $this->assertEquals($users[8]->id, $noncontacts[3]->id); + + // Verify the correct conversations were returned for the non-contacts. + $this->assertCount(0, $noncontacts[0]->conversations); + + $this->assertCount(1, $noncontacts[1]->conversations); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + $noncontacts[1]->conversations[$ic2->id]->type); + + $this->assertCount(1, $noncontacts[2]->conversations); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[2]->conversations[$gc1->id]->type); + + $this->assertCount(1, $noncontacts[3]->conversations); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[3]->conversations[$gc1->id]->type); + } + + /** + * Verify searching for users works even if no matching users from either contacts, or non-contacts can be found. + */ + public function test_message_search_users_with_empty_result() { + $this->resetAfterTest(); + + // Create some users, but make sure neither will match the search term. + $user1 = new stdClass(); + $user1->firstname = 'User'; + $user1->lastname = 'One'; + $user1 = $this->getDataGenerator()->create_user($user1); + $user2 = new stdClass(); + $user2->firstname = 'User'; + $user2->lastname = 'Two'; + $user2 = $this->getDataGenerator()->create_user($user2); + + // Perform a search as user1. + $this->setUser($user1); + $result = \core_message\api::message_search_users($user1->id, 'search'); + + // Check results are empty. + $this->assertCount(0, $result[0]); + $this->assertCount(0, $result[1]); + } + + /** + * Test verifying that limits and offsets work for both the contacts and non-contacts return data. + */ + public function test_message_search_users_limit_offset() { + $this->resetAfterTest(); + + // Create 20 users. + $users = []; + foreach (range(1, 20) as $i) { + $user = new stdClass(); + $user->firstname = "User search"; + $user->lastname = $i; + $user = $this->getDataGenerator()->create_user($user); + $users[$i] = $user; + } + + // Enrol the first 9 users in the same course, but leave them as non-contacts. + $this->setAdminUser(); + $course1 = $this->getDataGenerator()->create_course(); + foreach (range(1, 9) as $i) { + $this->getDataGenerator()->enrol_user($users[$i]->id, $course1->id); + } + + // Add 5 users, starting at the 11th user, as contacts for user1. + foreach (range(11, 15) as $i) { + \core_message\api::add_contact($users[1]->id, $users[$i]->id); + } + + // Set as the user performing the search. + $this->setUser($users[1]); + + // Search using a limit of 3. + // This tests the case where we have more results than the limit for both contacts and non-contacts. + $result = \core_message\api::message_search_users($users[1]->id, 'search', 0, 3); + $contacts = $result[0]; + $noncontacts = $result[1]; + + // Check that we retrieved the correct contacts. + $this->assertCount(3, $contacts); + $this->assertEquals($users[11]->id, $contacts[0]->id); + $this->assertEquals($users[12]->id, $contacts[1]->id); + $this->assertEquals($users[13]->id, $contacts[2]->id); // Check that we retrieved the correct non-contacts. $this->assertCount(3, $noncontacts); - $this->assertEquals($user5->id, $noncontacts[0]->id); - $this->assertEquals($user7->id, $noncontacts[1]->id); - $this->assertEquals($user6->id, $noncontacts[2]->id); + $this->assertEquals($users[2]->id, $noncontacts[0]->id); + $this->assertEquals($users[3]->id, $noncontacts[1]->id); + $this->assertEquals($users[4]->id, $noncontacts[2]->id); - // Perform a search $CFG->messagingallusers setting disabled. + // Now, offset to get the next batch of results. + // We expect to see 2 contacts, and 3 non-contacts. + $result = \core_message\api::message_search_users($users[1]->id, 'search', 3, 3); + $contacts = $result[0]; + $noncontacts = $result[1]; + $this->assertCount(2, $contacts); + $this->assertEquals($users[14]->id, $contacts[0]->id); + $this->assertEquals($users[15]->id, $contacts[1]->id); + + $this->assertCount(3, $noncontacts); + $this->assertEquals($users[5]->id, $noncontacts[0]->id); + $this->assertEquals($users[6]->id, $noncontacts[1]->id); + $this->assertEquals($users[7]->id, $noncontacts[2]->id); + + // Now, offset to get the next batch of results. + // We expect to see 0 contacts, and 2 non-contacts. + $result = \core_message\api::message_search_users($users[1]->id, 'search', 6, 3); + $contacts = $result[0]; + $noncontacts = $result[1]; + $this->assertCount(0, $contacts); + + $this->assertCount(2, $noncontacts); + $this->assertEquals($users[8]->id, $noncontacts[0]->id); + $this->assertEquals($users[9]->id, $noncontacts[1]->id); + } + + /** + * Tests searching users as a user having the 'moodle/user:viewdetails' capability. + */ + public function test_message_search_users_with_cap() { + $this->resetAfterTest(); + global $DB; + + // Create some users. + $users = []; + foreach (range(1, 8) as $i) { + $user = new stdClass(); + $user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term. + $user->lastname = $i; + $user = $this->getDataGenerator()->create_user($user); + $users[$i] = $user; + } + + // Enrol a few users in the same course, but leave them as non-contacts. + $course1 = $this->getDataGenerator()->create_course(); + $this->setAdminUser(); + $this->getDataGenerator()->enrol_user($users[1]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[6]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[7]->id, $course1->id); + + // Add some other users as contacts. + \core_message\api::add_contact($users[1]->id, $users[2]->id); + \core_message\api::add_contact($users[3]->id, $users[1]->id); + \core_message\api::add_contact($users[1]->id, $users[4]->id); + + // Set as the user performing the search. + $this->setUser($users[1]); + + // Grant the authenticated user role the capability 'user:viewdetails' at site context. + $authenticatedrole = $DB->get_record('role', ['shortname' => 'user'], '*', MUST_EXIST); + assign_capability('moodle/user:viewdetails', CAP_ALLOW, $authenticatedrole->id, context_system::instance()); + + // Perform a search with $CFG->messagingallusers disabled. set_config('messagingallusers', 0); - list($contacts, $noncontacts) = \core_message\api::message_search_users($user1->id, 'search'); + $result = \core_message\api::message_search_users($users[1]->id, 'search'); + $contacts = $result[0]; + $noncontacts = $result[1]; // Check that we retrieved the correct contacts. $this->assertCount(2, $contacts); - $this->assertEquals($user3->id, $contacts[0]->id); - $this->assertEquals($user2->id, $contacts[1]->id); + $this->assertEquals($users[2]->id, $contacts[0]->id); + $this->assertEquals($users[3]->id, $contacts[1]->id); // Check that we retrieved the correct non-contacts. - $this->assertCount(2, $noncontacts); - $this->assertEquals($user7->id, $noncontacts[0]->id); - $this->assertEquals($user6->id, $noncontacts[1]->id); + // Site-wide messaging is disabled, but since we can see all users, we expect to be able to search for any users. + $this->assertCount(4, $noncontacts); + $this->assertEquals($users[5]->id, $noncontacts[0]->id); + $this->assertEquals($users[6]->id, $noncontacts[1]->id); + $this->assertEquals($users[7]->id, $noncontacts[2]->id); + $this->assertEquals($users[8]->id, $noncontacts[3]->id); + } + + /** + * Tests searching users with messaging disabled. + */ + public function test_message_search_users_messaging_disabled() { + $this->resetAfterTest(); + + // Create a user. + $user = $this->getDataGenerator()->create_user(); + + // Disable messaging. + set_config('messaging', 0); + + // Ensure an exception is thrown. + $this->expectException('moodle_exception'); + \core_message\api::message_search_users($user->id, 'User'); } /** @@ -453,101 +712,6 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertCount(0, \core_message\api::get_conversations_between_users($user6->id, $user1->id)); } - /** - * Tests searching users with and without conversations. - */ - public function test_message_search_users_with_and_without_conversations() { - // Create some users. - $user1 = new stdClass(); - $user1->firstname = 'User search'; - $user1->lastname = 'One'; - $user1 = self::getDataGenerator()->create_user($user1); - - // Set as the user performing the search. - $this->setUser($user1); - - $user2 = new stdClass(); - $user2->firstname = 'User search'; - $user2->lastname = 'Two'; - $user2 = self::getDataGenerator()->create_user($user2); - - $user3 = new stdClass(); - $user3->firstname = 'User search'; - $user3->lastname = 'Three'; - $user3 = self::getDataGenerator()->create_user($user3); - - $user4 = new stdClass(); - $user4->firstname = 'User'; - $user4->lastname = 'Four'; - $user4 = self::getDataGenerator()->create_user($user4); - - $user5 = new stdClass(); - $user5->firstname = 'User search'; - $user5->lastname = 'Five'; - $user5 = self::getDataGenerator()->create_user($user5); - - // Add a user as contact. - \core_message\api::add_contact($user1->id, $user2->id); - - // Create private conversations with some users. - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user1->id, $user2->id)); - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user3->id, $user1->id)); - - // Create a group conversation with users. - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, - array($user1->id, $user2->id, $user4->id), - 'Project chat'); - - // Perform a search $CFG->messagingallusers setting enabled. - set_config('messagingallusers', 1); - list($contacts, $noncontacts) = \core_message\api::message_search_users($user1->id, 'search'); - - // Check that we retrieved the correct contacts. - $this->assertCount(1, $contacts); - - // Check that we retrieved the correct conversations for contacts. - $this->assertCount(2, $contacts[0]->conversations); - - // Check that we retrieved the correct non-contacts. - $this->assertCount(2, $noncontacts); - $this->assertEquals($user5->id, $noncontacts[0]->id); - $this->assertEquals($user3->id, $noncontacts[1]->id); - - // Check that we retrieved the correct conversations for non-contacts. - $this->assertCount(0, $noncontacts[0]->conversations); - $this->assertCount(1, $noncontacts[1]->conversations); - } - - /** - * Tests searching users with empty result. - */ - public function test_message_search_users_with_empty_result() { - - // Create some users. - $user1 = new stdClass(); - $user1->firstname = 'User'; - $user1->lastname = 'One'; - $user1 = self::getDataGenerator()->create_user($user1); - - // Set as the user performing the search. - $this->setUser($user1); - - $user2 = new stdClass(); - $user2->firstname = 'User'; - $user2->lastname = 'Two'; - $user2 = self::getDataGenerator()->create_user($user2); - - // Perform a search $CFG->messagingallusers setting enabled. - set_config('messagingallusers', 1); - list($contacts, $noncontacts) = \core_message\api::message_search_users($user1->id, 'search'); - - // Check results are empty. - $this->assertEquals(0, count($contacts)); - $this->assertEquals(0, count($noncontacts)); - } - /** * Tests searching messages. */ diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index 54fe35d2d2e..9d2f0add5ab 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -2251,217 +2251,353 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { } /** - * Tests searching users. + * Tests searching for users when site-wide messaging is disabled. + * + * This test verifies that any contacts are returned, as well as any non-contacts whose profile we can view. + * If checks this by placing some users in the same course, where default caps would permit a user to view another user's + * profile. */ - public function test_message_search_users() { - $this->resetAfterTest(true); + public function test_message_search_users_messagingallusers_disabled() { + $this->resetAfterTest(); // Create some users. - $user1 = new stdClass(); - $user1->firstname = 'User search'; - $user1->lastname = 'One'; - $user1 = self::getDataGenerator()->create_user($user1); - // Set as the user performing the search. - $this->setUser($user1); + $users = []; + foreach (range(1, 7) as $i) { + $user = new stdClass(); + $user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term. + $user->lastname = $i; + $user = $this->getDataGenerator()->create_user($user); + $users[$i] = $user; + } - $user2 = new stdClass(); - $user2->firstname = 'User search'; - $user2->lastname = 'Two'; - $user2 = self::getDataGenerator()->create_user($user2); - - $user3 = new stdClass(); - $user3->firstname = 'User search'; - $user3->lastname = 'Three'; - $user3 = self::getDataGenerator()->create_user($user3); - - $user4 = new stdClass(); - $user4->firstname = 'User'; - $user4->lastname = 'Four'; - $user4 = self::getDataGenerator()->create_user($user4); - - $user5 = new stdClass(); - $user5->firstname = 'User search'; - $user5->lastname = 'Five'; - $user5 = self::getDataGenerator()->create_user($user5); - - $user6 = new stdClass(); - $user6->firstname = 'User search'; - $user6->lastname = 'Six'; - $user6 = self::getDataGenerator()->create_user($user6); - - $user7 = new stdClass(); - $user7->firstname = 'User search'; - $user7->lastname = 'Seven'; - $user7 = self::getDataGenerator()->create_user($user7); - - // Add some users as contacts. - \core_message\api::add_contact($user1->id, $user2->id); - \core_message\api::add_contact($user3->id, $user1->id); - \core_message\api::add_contact($user1->id, $user4->id); - - // Create private conversations with some users. - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user1->id, $user6->id)); - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user7->id, $user1->id)); - - // Perform a search $CFG->messagingallusers setting enabled. - set_config('messagingallusers', 1); - $result = core_message_external::message_search_users($user1->id, 'search'); - - // We need to execute the return values cleaning process to simulate the web service server. - $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), - $result); - - // Confirm that we returns contacts and non-contacts. - $contacts = $result['contacts']; - $noncontacts = $result['noncontacts']; - - // Check that we retrieved the correct contacts. - $this->assertCount(2, $contacts); - $this->assertEquals($user3->id, $contacts[0]['id']); - $this->assertEquals($user2->id, $contacts[1]['id']); - - // Check that we retrieved the correct non-contacts. - $this->assertCount(3, $noncontacts); - $this->assertEquals($user5->id, $noncontacts[0]['id']); - $this->assertEquals($user7->id, $noncontacts[1]['id']); - $this->assertEquals($user6->id, $noncontacts[2]['id']); - - // Perform a search $CFG->messagingallusers setting disabled. - set_config('messagingallusers', 0); - $result = core_message_external::message_search_users($user1->id, 'search'); - - // We need to execute the return values cleaning process to simulate the web service server. - $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), - $result); - - // Confirm that we returns contacts and non-contacts. - $contacts = $result['contacts']; - $noncontacts = $result['noncontacts']; - - // Check that we retrieved the correct contacts. - $this->assertCount(2, $contacts); - $this->assertEquals($user3->id, $contacts[0]['id']); - $this->assertEquals($user2->id, $contacts[1]['id']); - - // Check that we retrieved the correct non-contacts. - $this->assertCount(2, $noncontacts); - $this->assertEquals($user7->id, $noncontacts[0]['id']); - $this->assertEquals($user6->id, $noncontacts[1]['id']); - } - - /** - * Tests searching users as another user. - */ - public function test_message_search_users_as_other_user() { - $this->resetAfterTest(true); - - // The person doing the search. + // Enrol a few users in the same course, but leave them as non-contacts. + $course1 = $this->getDataGenerator()->create_course(); $this->setAdminUser(); + $this->getDataGenerator()->enrol_user($users[1]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[6]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[7]->id, $course1->id); - // Create some users. - $user1 = new stdClass(); - $user1->firstname = 'User search'; - $user1->lastname = 'One'; - $user1 = self::getDataGenerator()->create_user($user1); + // Add some other users as contacts. + \core_message\api::add_contact($users[1]->id, $users[2]->id); + \core_message\api::add_contact($users[3]->id, $users[1]->id); + \core_message\api::add_contact($users[1]->id, $users[4]->id); - $user2 = new stdClass(); - $user2->firstname = 'User search'; - $user2->lastname = 'Two'; - $user2 = self::getDataGenerator()->create_user($user2); - - $user3 = new stdClass(); - $user3->firstname = 'User search'; - $user3->lastname = 'Three'; - $user3 = self::getDataGenerator()->create_user($user3); - - $user4 = new stdClass(); - $user4->firstname = 'User'; - $user4->lastname = 'Four'; - $user4 = self::getDataGenerator()->create_user($user4); - - $user5 = new stdClass(); - $user5->firstname = 'User search'; - $user5->lastname = 'Five'; - $user5 = self::getDataGenerator()->create_user($user5); - - $user6 = new stdClass(); - $user6->firstname = 'User search'; - $user6->lastname = 'Six'; - $user6 = self::getDataGenerator()->create_user($user6); - - $user7 = new stdClass(); - $user7->firstname = 'User search'; - $user7->lastname = 'Seven'; - $user7 = self::getDataGenerator()->create_user($user7); - - // Add some users as contacts. - \core_message\api::add_contact($user1->id, $user2->id); - \core_message\api::add_contact($user3->id, $user1->id); - \core_message\api::add_contact($user1->id, $user4->id); - - // Create private conversations with some users. + // Create individual conversations between some users, one contact and one non-contact. \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user1->id, $user6->id)); + [$users[1]->id, $users[2]->id]); \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user7->id, $user1->id)); + [$users[6]->id, $users[1]->id]); - // Perform a search $CFG->messagingallusers setting enabled. - set_config('messagingallusers', 1); - $result = core_message_external::message_search_users($user1->id, 'search'); + // Create a group conversation between 4 users, including a contact and a non-contact. + \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$users[1]->id, $users[2]->id, $users[4]->id, $users[7]->id], 'Project chat'); - // We need to execute the return values cleaning process to simulate the web service server. - $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), - $result); + // Set as the user performing the search. + $this->setUser($users[1]); - // Confirm that we returns contacts and non-contacts. - $contacts = $result['contacts']; - $noncontacts = $result['noncontacts']; - - // Check that we retrieved the correct contacts. - $this->assertCount(2, $contacts); - $this->assertEquals($user3->id, $contacts[0]['id']); - $this->assertEquals($user2->id, $contacts[1]['id']); - - // Check that we retrieved the correct non-contacts. - $this->assertCount(3, $noncontacts); - $this->assertEquals($user5->id, $noncontacts[0]['id']); - $this->assertEquals($user7->id, $noncontacts[1]['id']); - $this->assertEquals($user6->id, $noncontacts[2]['id']); - - // Perform a search $CFG->messagingallusers setting disabled. + // Perform a search with $CFG->messagingallusers disabled. set_config('messagingallusers', 0); - $result = core_message_external::message_search_users($user1->id, 'search'); - - // We need to execute the return values cleaning process to simulate the web service server. - $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), - $result); + $result = core_message_external::message_search_users($users[1]->id, 'search'); + $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), $result); // Confirm that we returns contacts and non-contacts. + $this->assertArrayHasKey('contacts', $result); + $this->assertArrayHasKey('noncontacts', $result); $contacts = $result['contacts']; $noncontacts = $result['noncontacts']; // Check that we retrieved the correct contacts. $this->assertCount(2, $contacts); - $this->assertEquals($user3->id, $contacts[0]['id']); - $this->assertEquals($user2->id, $contacts[1]['id']); + $this->assertEquals($users[2]->id, $contacts[0]['id']); + $this->assertEquals($users[3]->id, $contacts[1]['id']); + + // Verify the correct conversations were returned for the contacts. + $this->assertCount(2, $contacts[0]['conversations']); + // We can't rely on the ordering of conversations within the results, so sort by id first. + usort($contacts[0]['conversations'], function($a, $b) { + return $a['id'] < $b['id']; + }); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $contacts[0]['conversations'][0]['type']); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $contacts[0]['conversations'][1]['type']); + + $this->assertCount(0, $contacts[1]['conversations']); // Check that we retrieved the correct non-contacts. + // When site wide messaging is disabled, we expect to see only those users whose profiles we can view. $this->assertCount(2, $noncontacts); - $this->assertEquals($user7->id, $noncontacts[0]['id']); - $this->assertEquals($user6->id, $noncontacts[1]['id']); + $this->assertEquals($users[6]->id, $noncontacts[0]['id']); + $this->assertEquals($users[7]->id, $noncontacts[1]['id']); + + // Verify the correct conversations were returned for the non-contacts. + $this->assertCount(1, $noncontacts[0]['conversations']); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $noncontacts[0]['conversations'][0]['type']); + + $this->assertCount(1, $noncontacts[1]['conversations']); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[1]['conversations'][0]['type']); } /** - * Tests searching users as another user without the proper capabilities. + * Tests searching for users when site-wide messaging is enabled. + * + * This test verifies that any contacts are returned, as well as any non-contacts, regardless of whether the searching user + * can view their respective profile. */ - public function test_message_search_users_as_other_user_without_cap() { - $this->resetAfterTest(true); + public function test_message_search_users_messagingallusers_enabled() { + $this->resetAfterTest(); // Create some users. - $user1 = self::getDataGenerator()->create_user(); - $user2 = self::getDataGenerator()->create_user(); + $users = []; + foreach (range(1, 8) as $i) { + $user = new stdClass(); + $user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term. + $user->lastname = $i; + $user = $this->getDataGenerator()->create_user($user); + $users[$i] = $user; + } + + // Enrol a few users in the same course, but leave them as non-contacts. + $course1 = $this->getDataGenerator()->create_course(); + $this->setAdminUser(); + $this->getDataGenerator()->enrol_user($users[1]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[6]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[7]->id, $course1->id); + + // Add some other users as contacts. + \core_message\api::add_contact($users[1]->id, $users[2]->id); + \core_message\api::add_contact($users[3]->id, $users[1]->id); + \core_message\api::add_contact($users[1]->id, $users[4]->id); + + // Create individual conversations between some users, one contact and one non-contact. + \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$users[1]->id, $users[2]->id]); + \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [$users[6]->id, $users[1]->id]); + + // Create a group conversation between 5 users, including a contact and a non-contact, and a user NOT in a shared course. + \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [$users[1]->id, $users[2]->id, $users[4]->id, $users[7]->id, $users[8]->id], 'Project chat'); + + // Set as the user performing the search. + $this->setUser($users[1]); + + // Perform a search with $CFG->messagingallusers enabled. + set_config('messagingallusers', 1); + $result = core_message_external::message_search_users($users[1]->id, 'search'); + $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), $result); + + // Confirm that we returns contacts and non-contacts. + $this->assertArrayHasKey('contacts', $result); + $this->assertArrayHasKey('noncontacts', $result); + $contacts = $result['contacts']; + $noncontacts = $result['noncontacts']; + + // Check that we retrieved the correct contacts. + $this->assertCount(2, $contacts); + $this->assertEquals($users[2]->id, $contacts[0]['id']); + $this->assertEquals($users[3]->id, $contacts[1]['id']); + + // Verify the correct conversations were returned for the contacts. + $this->assertCount(2, $contacts[0]['conversations']); + // We can't rely on the ordering of conversations within the results, so sort by id first. + usort($contacts[0]['conversations'], function($a, $b) { + return $a['id'] < $b['id']; + }); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $contacts[0]['conversations'][0]['type']); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $contacts[0]['conversations'][1]['type']); + + $this->assertCount(0, $contacts[1]['conversations']); + + // Check that we retrieved the correct non-contacts. + // If site wide messaging is enabled, we expect to be able to search for any users. + $this->assertCount(4, $noncontacts); + $this->assertEquals($users[5]->id, $noncontacts[0]['id']); + $this->assertEquals($users[6]->id, $noncontacts[1]['id']); + $this->assertEquals($users[7]->id, $noncontacts[2]['id']); + $this->assertEquals($users[8]->id, $noncontacts[3]['id']); + + // Verify the correct conversations were returned for the non-contacts. + $this->assertCount(0, $noncontacts[0]['conversations']); + + $this->assertCount(1, $noncontacts[1]['conversations']); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $noncontacts[1]['conversations'][0]['type']); + + $this->assertCount(1, $noncontacts[2]['conversations']); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[2]['conversations'][0]['type']); + + $this->assertCount(1, $noncontacts[3]['conversations']); + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[3]['conversations'][0]['type']); + } + + /** + * Verify searching for users works even if no matching users from either contacts, or non-contacts can be found. + */ + public function test_message_search_users_with_empty_result() { + $this->resetAfterTest(); + + // Create some users, but make sure neither will match the search term. + $user1 = new stdClass(); + $user1->firstname = 'User'; + $user1->lastname = 'One'; + $user1 = $this->getDataGenerator()->create_user($user1); + $user2 = new stdClass(); + $user2->firstname = 'User'; + $user2->lastname = 'Two'; + $user2 = $this->getDataGenerator()->create_user($user2); + + // Perform a search as user1. + $this->setUser($user1); + $result = core_message_external::message_search_users($user1->id, 'search'); + $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), $result); + + // Check results are empty. + $this->assertCount(0, $result['contacts']); + $this->assertCount(0, $result['noncontacts']); + } + + /** + * Test verifying that limits and offsets work for both the contacts and non-contacts return data. + */ + public function test_message_search_users_limit_offset() { + $this->resetAfterTest(); + + // Create 20 users. + $users = []; + foreach (range(1, 20) as $i) { + $user = new stdClass(); + $user->firstname = "User search"; + $user->lastname = $i; + $user = $this->getDataGenerator()->create_user($user); + $users[$i] = $user; + } + + // Enrol the first 9 users in the same course, but leave them as non-contacts. + $this->setAdminUser(); + $course1 = $this->getDataGenerator()->create_course(); + foreach (range(1, 9) as $i) { + $this->getDataGenerator()->enrol_user($users[$i]->id, $course1->id); + } + + // Add 5 users, starting at the 11th user, as contacts for user1. + foreach (range(11, 15) as $i) { + \core_message\api::add_contact($users[1]->id, $users[$i]->id); + } + + // Set as the user performing the search. + $this->setUser($users[1]); + + // Search using a limit of 3. + // This tests the case where we have more results than the limit for both contacts and non-contacts. + $result = core_message_external::message_search_users($users[1]->id, 'search', 0, 3); + $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), $result); + $contacts = $result['contacts']; + $noncontacts = $result['noncontacts']; + + // Check that we retrieved the correct contacts. + $this->assertCount(3, $contacts); + $this->assertEquals($users[11]->id, $contacts[0]['id']); + $this->assertEquals($users[12]->id, $contacts[1]['id']); + $this->assertEquals($users[13]->id, $contacts[2]['id']); + + // Check that we retrieved the correct non-contacts. + $this->assertCount(3, $noncontacts); + $this->assertEquals($users[2]->id, $noncontacts[0]['id']); + $this->assertEquals($users[3]->id, $noncontacts[1]['id']); + $this->assertEquals($users[4]->id, $noncontacts[2]['id']); + + // Now, offset to get the next batch of results. + // We expect to see 2 contacts, and 3 non-contacts. + $result = core_message_external::message_search_users($users[1]->id, 'search', 3, 3); + $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), $result); + $contacts = $result['contacts']; + $noncontacts = $result['noncontacts']; + $this->assertCount(2, $contacts); + $this->assertEquals($users[14]->id, $contacts[0]['id']); + $this->assertEquals($users[15]->id, $contacts[1]['id']); + + $this->assertCount(3, $noncontacts); + $this->assertEquals($users[5]->id, $noncontacts[0]['id']); + $this->assertEquals($users[6]->id, $noncontacts[1]['id']); + $this->assertEquals($users[7]->id, $noncontacts[2]['id']); + + // Now, offset to get the next batch of results. + // We expect to see 0 contacts, and 2 non-contacts. + $result = core_message_external::message_search_users($users[1]->id, 'search', 6, 3); + $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), $result); + $contacts = $result['contacts']; + $noncontacts = $result['noncontacts']; + $this->assertCount(0, $contacts); + + $this->assertCount(2, $noncontacts); + $this->assertEquals($users[8]->id, $noncontacts[0]['id']); + $this->assertEquals($users[9]->id, $noncontacts[1]['id']); + } + + /** + * Tests searching users as another user having the 'moodle/user:viewdetails' capability. + */ + public function test_message_search_users_with_cap() { + $this->resetAfterTest(); + global $DB; + + // Create some users. + $users = []; + foreach (range(1, 8) as $i) { + $user = new stdClass(); + $user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term. + $user->lastname = $i; + $user = $this->getDataGenerator()->create_user($user); + $users[$i] = $user; + } + + // Enrol a few users in the same course, but leave them as non-contacts. + $course1 = $this->getDataGenerator()->create_course(); + $this->setAdminUser(); + $this->getDataGenerator()->enrol_user($users[1]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[6]->id, $course1->id); + $this->getDataGenerator()->enrol_user($users[7]->id, $course1->id); + + // Add some other users as contacts. + \core_message\api::add_contact($users[1]->id, $users[2]->id); + \core_message\api::add_contact($users[3]->id, $users[1]->id); + \core_message\api::add_contact($users[1]->id, $users[4]->id); + + // Set as the user performing the search. + $this->setUser($users[1]); + + // Grant the authenticated user role the capability 'user:viewdetails' at site context. + $authenticatedrole = $DB->get_record('role', ['shortname' => 'user'], '*', MUST_EXIST); + assign_capability('moodle/user:viewdetails', CAP_ALLOW, $authenticatedrole->id, context_system::instance()); + + // Perform a search with $CFG->messagingallusers disabled. + set_config('messagingallusers', 0); + $result = core_message_external::message_search_users($users[1]->id, 'search'); + $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), $result); + $contacts = $result['contacts']; + $noncontacts = $result['noncontacts']; + + // Check that we retrieved the correct contacts. + $this->assertCount(2, $contacts); + $this->assertEquals($users[2]->id, $contacts[0]['id']); + $this->assertEquals($users[3]->id, $contacts[1]['id']); + + // Check that we retrieved the correct non-contacts. + // Site-wide messaging is disabled, but since we can see all users, we expect to be able to search for any users. + $this->assertCount(4, $noncontacts); + $this->assertEquals($users[5]->id, $noncontacts[0]['id']); + $this->assertEquals($users[6]->id, $noncontacts[1]['id']); + $this->assertEquals($users[7]->id, $noncontacts[2]['id']); + $this->assertEquals($users[8]->id, $noncontacts[3]['id']); + } + + /** + * Tests searching users as another user without the 'moodle/user:viewdetails' capability. + */ + public function test_message_search_users_without_cap() { + $this->resetAfterTest(); + + // Create some users. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); // The person doing the search for another user. $this->setUser($user1); @@ -2472,91 +2608,14 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertDebuggingCalled(); } - /** - * Tests searching users with and without conversations. - */ - public function test_message_search_users_with_and_without_conversations() { - $this->resetAfterTest(true); - - // Create some users. - $user1 = new stdClass(); - $user1->firstname = 'User search'; - $user1->lastname = 'One'; - $user1 = self::getDataGenerator()->create_user($user1); - - // Set as the user performing the search. - $this->setUser($user1); - - $user2 = new stdClass(); - $user2->firstname = 'User search'; - $user2->lastname = 'Two'; - $user2 = self::getDataGenerator()->create_user($user2); - - $user3 = new stdClass(); - $user3->firstname = 'User search'; - $user3->lastname = 'Three'; - $user3 = self::getDataGenerator()->create_user($user3); - - $user4 = new stdClass(); - $user4->firstname = 'User'; - $user4->lastname = 'Four'; - $user4 = self::getDataGenerator()->create_user($user4); - - $user5 = new stdClass(); - $user5->firstname = 'User search'; - $user5->lastname = 'Five'; - $user5 = self::getDataGenerator()->create_user($user5); - - // Add a user as contact. - \core_message\api::add_contact($user1->id, $user2->id); - - // Create private conversations with some users. - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user1->id, $user2->id)); - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - array($user3->id, $user1->id)); - - // Create a group conversation with users. - \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, - array($user1->id, $user2->id, $user4->id), - 'Project chat'); - - // Perform a search $CFG->messagingallusers setting enabled. - set_config('messagingallusers', 1); - $result = core_message_external::message_search_users($user1->id, 'search'); - - // We need to execute the return values cleaning process to simulate the web service server. - $result = external_api::clean_returnvalue(core_message_external::message_search_users_returns(), - $result); - - // Confirm that we returns contacts and non-contacts. - $contacts = $result['contacts']; - $noncontacts = $result['noncontacts']; - - // Check that we retrieved the correct contacts. - $this->assertCount(1, $contacts); - - // Check that we retrieved the correct conversations for contacts. - $this->assertCount(2, $contacts[0]['conversations']); - - // Check that we retrieved the correct non-contacts. - $this->assertCount(2, $noncontacts); - $this->assertEquals($user5->id, $noncontacts[0]['id']); - $this->assertEquals($user3->id, $noncontacts[1]['id']); - - // Check that we retrieved the correct conversations for non-contacts. - $this->assertCount(0, $noncontacts[0]['conversations']); - $this->assertCount(1, $noncontacts[1]['conversations']); - } - /** * Tests searching users with messaging disabled. */ public function test_message_search_users_messaging_disabled() { - $this->resetAfterTest(true); + $this->resetAfterTest(); // Create some skeleton data just so we can call the WS. - $user = self::getDataGenerator()->create_user(); + $user = $this->getDataGenerator()->create_user(); // The person doing the search. $this->setUser($user); @@ -2567,7 +2626,6 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { // Ensure an exception is thrown. $this->expectException('moodle_exception'); core_message_external::message_search_users($user->id, 'User'); - $this->assertDebuggingCalled(); } /**