diff --git a/message/classes/api.php b/message/classes/api.php index ddf2f4b773d..9788b21ced2 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -1960,19 +1960,65 @@ class api { public static function get_conversation_between_users(array $userids) { global $DB; - $hash = helper::get_conversation_hash($userids); + $conversations = self::get_individual_conversations_between_users([$userids]); + $conversation = $conversations[0]; - $params = [ - 'type' => self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, - 'convhash' => $hash - ]; - if ($conversation = $DB->get_record('message_conversations', $params)) { + if ($conversation) { return $conversation->id; } return false; } + /** + * Returns the conversations between sets of users. + * + * The returned array of results will be in the same order as the requested + * arguments, null will be returned if there is no conversation for that user + * pair. + * + * For example: + * If we have 6 users with ids 1, 2, 3, 4, 5, 6 where only 2 conversations + * exist. One between 1 and 2 and another between 5 and 6. + * + * Then if we call: + * $conversations = get_individual_conversations_between_users([[1,2], [3,4], [5,6]]); + * + * The conversations array will look like: + * [, null, ]; + * + * Where null is returned for the pairing of [3, 4] since no record exists. + * + * @param array $useridsets An array of arrays where the inner array is the set of user ids + * @return stdClass[] Array of conversation records + */ + public static function get_individual_conversations_between_users(array $useridsets) : array { + global $DB; + + if (empty($useridsets)) { + return []; + } + + $hashes = array_map(function($userids) { + return helper::get_conversation_hash($userids); + }, $useridsets); + + list($inorequalsql, $params) = $DB->get_in_or_equal($hashes); + array_unshift($params, self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL); + $where = "type = ? AND convhash ${inorequalsql}"; + $conversations = array_fill(0, count($hashes), null); + $records = $DB->get_records_select('message_conversations', $where, $params); + + foreach (array_values($records) as $record) { + $index = array_search($record->convhash, $hashes); + if ($index !== false) { + $conversations[$index] = $record; + } + } + + return $conversations; + } + /** * Creates a conversation between two users. * @@ -2106,8 +2152,9 @@ class api { * * @param int $userid The id of the user who is creating the contact request * @param int $requesteduserid The id of the user being requested + * @return \stdClass the request */ - public static function create_contact_request(int $userid, int $requesteduserid) { + public static function create_contact_request(int $userid, int $requesteduserid) : \stdClass { global $DB; $request = new \stdClass(); @@ -2115,32 +2162,9 @@ class api { $request->requesteduserid = $requesteduserid; $request->timecreated = time(); - $DB->insert_record('message_contact_requests', $request); + $request->id = $DB->insert_record('message_contact_requests', $request); - // Send a notification. - $userfrom = \core_user::get_user($userid); - $userfromfullname = fullname($userfrom); - $userto = \core_user::get_user($requesteduserid); - $url = new \moodle_url('/message/pendingcontactrequests.php'); - - $subject = get_string('messagecontactrequestsnotificationsubject', 'core_message', $userfromfullname); - $fullmessage = get_string('messagecontactrequestsnotification', 'core_message', $userfromfullname); - - $message = new \core\message\message(); - $message->courseid = SITEID; - $message->component = 'moodle'; - $message->name = 'messagecontactrequests'; - $message->notification = 1; - $message->userfrom = $userfrom; - $message->userto = $userto; - $message->subject = $subject; - $message->fullmessage = text_to_html($fullmessage); - $message->fullmessageformat = FORMAT_HTML; - $message->fullmessagehtml = $fullmessage; - $message->smallmessage = ''; - $message->contexturl = $url->out(false); - - message_send($message); + return $request; } @@ -2207,6 +2231,17 @@ class api { return []; } + /** + * Count how many contact requests the user has received. + * + * @param \stdClass $user The user to fetch contact requests for + * @return int The count + */ + public static function count_received_contact_requests(\stdClass $user) : int { + global $DB; + return $DB->count_records('message_contact_requests', ['requesteduserid' => $user->id]); + } + /** * Handles adding a contact. * @@ -2354,6 +2389,23 @@ class api { return $DB->record_exists('message_users_blocked', ['userid' => $userid, 'blockeduserid' => $blockeduserid]); } + /** + * Get contact requests between users. + * + * @param int $userid The id of the user who is creating the contact request + * @param int $requesteduserid The id of the user being requested + * @return \stdClass[] + */ + public static function get_contact_requests_between_users(int $userid, int $requesteduserid) : array { + global $DB; + + $sql = "SELECT * + FROM {message_contact_requests} mcr + WHERE (mcr.userid = ? AND mcr.requesteduserid = ?) + OR (mcr.userid = ? AND mcr.requesteduserid = ?)"; + return $DB->get_records_sql($sql, [$userid, $requesteduserid, $requesteduserid, $userid]); + } + /** * Checks if a contact request already exists between users. * diff --git a/message/externallib.php b/message/externallib.php index b8a1dc16fb2..42c4491055f 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -842,21 +842,27 @@ class core_message_external extends external_api { throw new required_capability_exception($context, $capability, 'nopermissions', ''); } + $result = [ + 'warnings' => [] + ]; + if (!\core_message\api::can_create_contact($params['userid'], $params['requesteduserid'])) { - $warning[] = [ + $result['warnings'][] = [ 'item' => 'user', 'itemid' => $params['requesteduserid'], 'warningcode' => 'cannotcreatecontactrequest', 'message' => 'You are unable to create a contact request for this user' ]; - return $warning; + } else { + if ($requests = \core_message\api::get_contact_requests_between_users($params['userid'], $params['requesteduserid'])) { + // There should only ever be one but just in case there are multiple then we can return the first. + $result['request'] = array_shift($requests); + } else { + $result['request'] = \core_message\api::create_contact_request($params['userid'], $params['requesteduserid']); + } } - if (!\core_message\api::does_contact_request_exist($params['userid'], $params['requesteduserid'])) { - \core_message\api::create_contact_request($params['userid'], $params['requesteduserid']); - } - - return []; + return $result; } /** @@ -865,7 +871,21 @@ class core_message_external extends external_api { * @return external_description */ public static function create_contact_request_returns() { - return new external_warnings(); + return new external_single_structure( + array( + 'request' => new external_single_structure( + array( + 'id' => new external_value(PARAM_INT, 'Message id'), + 'userid' => new external_value(PARAM_INT, 'User from id'), + 'requesteduserid' => new external_value(PARAM_INT, 'User to id'), + 'timecreated' => new external_value(PARAM_INT, 'Time created'), + ), + 'request record', + VALUE_OPTIONAL + ), + 'warnings' => new external_warnings() + ) + ); } /** diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 30add021e0c..61fe9b04cba 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -4570,13 +4570,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); - \core_message\api::create_contact_request($user1->id, $user2->id); - - $request = $DB->get_records('message_contact_requests'); - - $this->assertCount(1, $request); - - $request = reset($request); + $request = \core_message\api::create_contact_request($user1->id, $user2->id); $this->assertEquals($user1->id, $request->userid); $this->assertEquals($user2->id, $request->requesteduserid); @@ -4628,6 +4622,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase { * Test retrieving contact requests. */ public function test_get_contact_requests() { + global $PAGE; + $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); $user3 = self::getDataGenerator()->create_user(); @@ -4643,6 +4639,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertCount(1, $requests); $request = reset($requests); + $userpicture = new \user_picture($user2); + $profileimageurl = $userpicture->get_url($PAGE)->out(false); $this->assertEquals($user2->id, $request->id); $this->assertEquals(fullname($user2), $request->fullname); @@ -4819,6 +4817,78 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertTrue(\core_message\api::does_contact_request_exist($user2->id, $user1->id)); } + /** + * Test the count_received_contact_requests() function. + */ + public function test_count_received_contact_requests() { + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + $user4 = self::getDataGenerator()->create_user(); + + $this->assertEquals(0, \core_message\api::count_received_contact_requests($user1)); + + \core_message\api::create_contact_request($user2->id, $user1->id); + + $this->assertEquals(1, \core_message\api::count_received_contact_requests($user1)); + + \core_message\api::create_contact_request($user3->id, $user1->id); + + $this->assertEquals(2, \core_message\api::count_received_contact_requests($user1)); + + \core_message\api::create_contact_request($user1->id, $user4->id); + // Function should ignore sent requests. + $this->assertEquals(2, \core_message\api::count_received_contact_requests($user1)); + } + + /** + * Test the get_contact_requests_between_users() function. + */ + public function test_get_contact_requests_between_users() { + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + $user4 = self::getDataGenerator()->create_user(); + + $this->assertEquals([], \core_message\api::get_contact_requests_between_users($user1->id, $user2->id)); + + $request1 = \core_message\api::create_contact_request($user2->id, $user1->id); + $results = \core_message\api::get_contact_requests_between_users($user1->id, $user2->id); + $results = array_values($results); + + $this->assertCount(1, $results); + $result = $results[0]; + $this->assertEquals($request1->id, $result->id); + + $request2 = \core_message\api::create_contact_request($user1->id, $user2->id); + $results = \core_message\api::get_contact_requests_between_users($user1->id, $user2->id); + $results = array_values($results); + + $this->assertCount(2, $results); + $actual = [(int) $results[0]->id, (int) $results[1]->id]; + $expected = [(int) $request1->id, (int) $request2->id]; + + sort($actual); + sort($expected); + + $this->assertEquals($expected, $actual); + + // Request from a different user. + \core_message\api::create_contact_request($user3->id, $user1->id); + + $results = \core_message\api::get_contact_requests_between_users($user1->id, $user2->id); + $results = array_values($results); + + $this->assertCount(2, $results); + $actual = [(int) $results[0]->id, (int) $results[1]->id]; + $expected = [(int) $request1->id, (int) $request2->id]; + + sort($actual); + sort($expected); + + $this->assertEquals($expected, $actual); + } + /** * Test the user in conversation check. */ @@ -5084,6 +5154,148 @@ class core_message_api_testcase extends core_message_messagelib_testcase { ); } + + /** + * Test an empty array returned when no args given. + */ + public function test_get_individual_conversations_between_users_no_user_sets() { + $this->assertEmpty(\core_message\api::get_individual_conversations_between_users([])); + } + + /** + * Test a conversation is not returned if there is none. + */ + public function test_get_individual_conversations_between_users_no_conversation() { + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + + $this->assertEquals( + [null], + \core_message\api::get_individual_conversations_between_users([[$user1->id, $user2->id]]) + ); + } + + /** + * Test the result set includes null if there is no conversation between users. + */ + public function test_get_individual_conversations_between_users_partial_conversations() { + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $user3 = $generator->create_user(); + $type = \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL; + + $conversation1 = \core_message\api::create_conversation($type, [$user1->id, $user2->id]); + $conversation2 = \core_message\api::create_conversation($type, [$user1->id, $user3->id]); + + $results = \core_message\api::get_individual_conversations_between_users([ + [$user1->id, $user2->id], + [$user2->id, $user3->id], + [$user1->id, $user3->id] + ]); + + $result = array_map(function($result) { + if ($result) { + return $result->id; + } else { + return $result; + } + }, $results); + + $this->assertEquals( + [$conversation1->id, null, $conversation2->id], + $result + ); + } + + /** + * Test all conversations are returned if each set has a conversation. + */ + public function test_get_individual_conversations_between_users_all_conversations() { + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $user3 = $generator->create_user(); + $type = \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL; + + $conversation1 = \core_message\api::create_conversation($type, [$user1->id, $user2->id]); + $conversation2 = \core_message\api::create_conversation($type, [$user2->id, $user3->id]); + $conversation3 = \core_message\api::create_conversation($type, [$user1->id, $user3->id]); + + $results = \core_message\api::get_individual_conversations_between_users([ + [$user1->id, $user2->id], + [$user2->id, $user3->id], + [$user1->id, $user3->id] + ]); + + $result = array_map(function($result) { + if ($result) { + return $result->id; + } else { + return $result; + } + }, $results); + + $this->assertEquals( + [$conversation1->id, $conversation2->id, $conversation3->id], + $result + ); + } + + /** + * Test that the results are ordered to match the order of the parameters. + */ + public function test_get_individual_conversations_between_users_ordering() { + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $user3 = $generator->create_user(); + $type = \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL; + + $conversation1 = \core_message\api::create_conversation($type, [$user1->id, $user2->id]); + $conversation2 = \core_message\api::create_conversation($type, [$user2->id, $user3->id]); + $conversation3 = \core_message\api::create_conversation($type, [$user1->id, $user3->id]); + + $results = \core_message\api::get_individual_conversations_between_users([ + [$user1->id, $user2->id], + [$user2->id, $user3->id], + [$user1->id, $user3->id] + ]); + + $result = array_map(function($result) { + if ($result) { + return $result->id; + } else { + return $result; + } + }, $results); + + $this->assertEquals( + [$conversation1->id, $conversation2->id, $conversation3->id], + $result + ); + + $results = \core_message\api::get_individual_conversations_between_users([ + [$user2->id, $user3->id], + [$user1->id, $user2->id], + [$user1->id, $user3->id] + ]); + + $result = array_map(function($result) { + if ($result) { + return $result->id; + } else { + return $result; + } + }, $results); + + $this->assertEquals( + [$conversation2->id, $conversation1->id, $conversation3->id], + $result + ); + } + /** * Test returning members in a conversation with no contact requests. */ diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index 9d2f0add5ab..42aed082022 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -552,6 +552,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { * Test getting contact requests. */ public function test_get_contact_requests() { + global $PAGE; + $this->resetAfterTest(); $user1 = self::getDataGenerator()->create_user(); @@ -572,6 +574,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertCount(1, $requests); $request = reset($requests); + $userpicture = new \user_picture($user2); + $profileimageurl = $userpicture->get_url($PAGE)->out(false); $this->assertEquals($user2->id, $request['id']); $this->assertEquals(fullname($user2), $request['fullname']); @@ -677,7 +681,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $return = core_message_external::create_contact_request($user1->id, $user2->id); $return = external_api::clean_returnvalue(core_message_external::create_contact_request_returns(), $return); - $this->assertEquals(array(), $return); + $this->assertEquals([], $return['warnings']); $request = $DB->get_records('message_contact_requests'); @@ -685,8 +689,10 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $request = reset($request); - $this->assertEquals($user1->id, $request->userid); - $this->assertEquals($user2->id, $request->requesteduserid); + $this->assertEquals($request->id, $return['request']['id']); + $this->assertEquals($request->userid, $return['request']['userid']); + $this->assertEquals($request->requesteduserid, $return['request']['requesteduserid']); + $this->assertEquals($request->timecreated, $return['request']['timecreated']); } /** @@ -707,7 +713,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $return = core_message_external::create_contact_request($user1->id, $user2->id); $return = external_api::clean_returnvalue(core_message_external::create_contact_request_returns(), $return); - $warning = reset($return); + $warning = reset($return['warnings']); $this->assertEquals('user', $warning['item']); $this->assertEquals($user2->id, $warning['itemid']); diff --git a/message/tests/privacy_provider_test.php b/message/tests/privacy_provider_test.php index 101adbba016..5005483f69b 100644 --- a/message/tests/privacy_provider_test.php +++ b/message/tests/privacy_provider_test.php @@ -693,8 +693,8 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide // There should be 4 conversation members. $this->assertEquals(4, $DB->count_records('message_conversation_members')); - // There should be 3 notifications + 2 for the contact request. - $this->assertEquals(5, $DB->count_records('notifications')); + // There should be 3 notifications. + $this->assertEquals(3, $DB->count_records('notifications')); provider::delete_data_for_all_users_in_context($user1context); @@ -732,8 +732,8 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide // And user1 is not in any conversation. $this->assertEquals(0, $DB->count_records('message_conversation_members', ['userid' => $user1->id])); - // Confirm there is only 1 notification + 1 for the contact request. - $this->assertEquals(2, $DB->count_records('notifications')); + // Confirm there is only 1 notification. + $this->assertEquals(1, $DB->count_records('notifications')); // And it is not related to user1. $this->assertEquals(0, $DB->count_records_select('notifications', 'useridfrom = ? OR useridto = ? ', [$user1->id, $user1->id])); @@ -800,8 +800,8 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide // There should be two conversation members. $this->assertEquals(2, $DB->count_records('message_conversation_members')); - // There should be three notifications + two for the contact requests. - $this->assertEquals(5, $DB->count_records('notifications')); + // There should be three notifications. + $this->assertEquals(3, $DB->count_records('notifications')); $user1context = context_user::instance($user1->id); $contextlist = new \core_privacy\local\request\approved_contextlist($user1, 'core_message', @@ -846,13 +846,9 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide $mcm = reset($mcms); $this->assertEquals($user2->id, $mcm->userid); - $this->assertCount(2, $notifications); + $this->assertCount(1, $notifications); ksort($notifications); - $notification = array_shift($notifications); - $this->assertEquals($user2->id, $notification->useridfrom); - $this->assertEquals($user4->id, $notification->useridto); - $notification = array_shift($notifications); $this->assertEquals($user2->id, $notification->useridfrom); $this->assertEquals($user3->id, $notification->useridto); @@ -1120,7 +1116,7 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide $this->assertEquals(2, $DB->count_records('message_conversation_members')); // There should be three notifications + two for the contact requests. - $this->assertEquals(5, $DB->count_records('notifications')); + $this->assertEquals(3, $DB->count_records('notifications')); $user1context = context_user::instance($user1->id); $approveduserlist = new \core_privacy\local\request\approved_userlist($user1context, 'core_message', @@ -1167,13 +1163,9 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide $mcm = reset($mcms); $this->assertEquals($user2->id, $mcm->userid); - $this->assertCount(2, $notifications); + $this->assertCount(1, $notifications); ksort($notifications); - $notification = array_shift($notifications); - $this->assertEquals($user2->id, $notification->useridfrom); - $this->assertEquals($user4->id, $notification->useridto); - $notification = array_shift($notifications); $this->assertEquals($user2->id, $notification->useridfrom); $this->assertEquals($user3->id, $notification->useridto);