From 6981de1080b7c74da9fd9b7e928ccdacd2ef06b3 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Fri, 2 Nov 2018 14:03:50 +0800 Subject: [PATCH 1/5] MDL-63850 core_message: prevent exception being thrown with empty array --- message/classes/helper.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/message/classes/helper.php b/message/classes/helper.php index 1927b643ee2..0ac16ec1666 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -489,6 +489,11 @@ class helper { public static function get_member_info(int $referenceuserid, array $userids) : array { global $DB, $PAGE; + // Prevent exception being thrown when array is empty. + if (empty($userids)) { + return []; + } + list($useridsql, $usersparams) = $DB->get_in_or_equal($userids); $userfields = \user_picture::fields('u', array('lastaccess')); $userssql = "SELECT $userfields, u.deleted, mc.id AS contactid, mub.id AS blockedid From 054834b00c925bc25bc02ddafb63f5daf3aaa253 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Fri, 2 Nov 2018 15:10:58 +0800 Subject: [PATCH 2/5] MDL-63850 core_message: moved contact request logic to helper --- message/classes/api.php | 20 +------------------- message/classes/helper.php | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/message/classes/api.php b/message/classes/api.php index 62995e31415..cc9ca69e829 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -2487,25 +2487,7 @@ class api { if ($members = $DB->get_records('message_conversation_members', ['conversationid' => $conversationid], 'timecreated ASC, id ASC', 'userid', $limitfrom, $limitnum)) { $userids = array_keys($members); - $members = helper::get_member_info($userid, $userids); - - // Check if we want to include contact requests as well. - if ($includecontactrequests) { - list($useridsql, $usersparams) = $DB->get_in_or_equal($userids); - - $wheresql = "(userid $useridsql OR requesteduserid $useridsql)"; - if ($contactrequests = $DB->get_records_select('message_contact_requests', $wheresql, - array_merge($usersparams, $usersparams), 'timecreated ASC, id ASC')) { - foreach ($contactrequests as $contactrequest) { - if (isset($members[$contactrequest->userid])) { - $members[$contactrequest->userid]->contactrequests[] = $contactrequest; - } - if (isset($members[$contactrequest->requesteduserid])) { - $members[$contactrequest->requesteduserid]->contactrequests[] = $contactrequest; - } - } - } - } + $members = helper::get_member_info($userid, $userids, $includecontactrequests); return $members; } diff --git a/message/classes/helper.php b/message/classes/helper.php index 0ac16ec1666..34204f118a7 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -482,11 +482,12 @@ class helper { * * @param int $referenceuserid the id of the user which check contact and blocked status. * @param array $userids + * @param bool $includecontactrequests Do we want to include contact requests with this data? * @return array the array of objects containing member info, indexed by userid. * @throws \coding_exception * @throws \dml_exception */ - public static function get_member_info(int $referenceuserid, array $userids) : array { + public static function get_member_info(int $referenceuserid, array $userids, bool $includecontactrequests = false) : array { global $DB, $PAGE; // Prevent exception being thrown when array is empty. @@ -532,6 +533,25 @@ class helper { $members[$data->id] = $data; } + + // Check if we want to include contact requests as well. + if (!empty($members) && $includecontactrequests) { + list($useridsql, $usersparams) = $DB->get_in_or_equal($userids); + + $wheresql = "(userid $useridsql OR requesteduserid $useridsql)"; + if ($contactrequests = $DB->get_records_select('message_contact_requests', $wheresql, + array_merge($usersparams, $usersparams), 'timecreated ASC, id ASC')) { + foreach ($contactrequests as $contactrequest) { + if (isset($members[$contactrequest->userid])) { + $members[$contactrequest->userid]->contactrequests[] = $contactrequest; + } + if (isset($members[$contactrequest->requesteduserid])) { + $members[$contactrequest->requesteduserid]->contactrequests[] = $contactrequest; + } + } + } + } + return $members; } From 5c675c507680a293712dbdd0cce84c6c980d29e8 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Tue, 6 Nov 2018 14:25:07 +0800 Subject: [PATCH 3/5] MDL-63850 core_message: '$referenceuserid' used when returning requests --- message/classes/helper.php | 7 ++++--- message/tests/api_test.php | 18 +++--------------- message/tests/externallib_test.php | 18 +++--------------- 3 files changed, 10 insertions(+), 33 deletions(-) diff --git a/message/classes/helper.php b/message/classes/helper.php index 34204f118a7..407c59b807e 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -538,9 +538,10 @@ class helper { if (!empty($members) && $includecontactrequests) { list($useridsql, $usersparams) = $DB->get_in_or_equal($userids); - $wheresql = "(userid $useridsql OR requesteduserid $useridsql)"; - if ($contactrequests = $DB->get_records_select('message_contact_requests', $wheresql, - array_merge($usersparams, $usersparams), 'timecreated ASC, id ASC')) { + $wheresql = "(userid $useridsql AND requesteduserid = ?) OR (userid = ? AND requesteduserid $useridsql)"; + $params = array_merge($usersparams, [$referenceuserid, $referenceuserid], $usersparams); + if ($contactrequests = $DB->get_records_select('message_contact_requests', $wheresql, $params, + 'timecreated ASC, id ASC')) { foreach ($contactrequests as $contactrequest) { if (isset($members[$contactrequest->userid])) { $members[$contactrequest->userid]->contactrequests[] = $contactrequest; diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 76f27779ac8..4e7e518295c 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -4503,7 +4503,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals(true, $member1->showonlinestatus); $this->assertEquals(false, $member1->iscontact); $this->assertEquals(false, $member1->isblocked); - $this->assertCount(3, $member1->contactrequests); + $this->assertCount(2, $member1->contactrequests); $this->assertEquals($user2->id, $member2->id); $this->assertEquals(fullname($user2), $member2->fullname); @@ -4511,7 +4511,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals(true, $member2->showonlinestatus); $this->assertEquals(true, $member2->iscontact); $this->assertEquals(false, $member2->isblocked); - $this->assertCount(2, $member2->contactrequests); + $this->assertCount(1, $member2->contactrequests); $this->assertEquals($user3->id, $member3->id); $this->assertEquals(fullname($user3), $member3->fullname); @@ -4519,12 +4519,11 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals(true, $member3->showonlinestatus); $this->assertEquals(false, $member3->iscontact); $this->assertEquals(true, $member3->isblocked); - $this->assertCount(2, $member3->contactrequests); + $this->assertCount(1, $member3->contactrequests); // Confirm the contact requests are OK. $request1 = array_shift($member1->contactrequests); $request2 = array_shift($member1->contactrequests); - $request3 = array_shift($member1->contactrequests); $this->assertEquals($user1->id, $request1->userid); $this->assertEquals($user2->id, $request1->requesteduserid); @@ -4532,26 +4531,15 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals($user1->id, $request2->userid); $this->assertEquals($user3->id, $request2->requesteduserid); - $this->assertEquals($user1->id, $request3->userid); - $this->assertEquals($user4->id, $request3->requesteduserid); - $request1 = array_shift($member2->contactrequests); - $request2 = array_shift($member2->contactrequests); $this->assertEquals($user1->id, $request1->userid); $this->assertEquals($user2->id, $request1->requesteduserid); - $this->assertEquals($user2->id, $request2->userid); - $this->assertEquals($user3->id, $request2->requesteduserid); - $request1 = array_shift($member3->contactrequests); - $request2 = array_shift($member3->contactrequests); $this->assertEquals($user1->id, $request1->userid); $this->assertEquals($user3->id, $request1->requesteduserid); - - $this->assertEquals($user2->id, $request2->userid); - $this->assertEquals($user3->id, $request2->requesteduserid); } /** diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index 0781adc416f..62ccac4ea99 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -5236,7 +5236,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(true, $member1->showonlinestatus); $this->assertEquals(false, $member1->iscontact); $this->assertEquals(false, $member1->isblocked); - $this->assertCount(3, $member1->contactrequests); + $this->assertCount(2, $member1->contactrequests); $this->assertEquals($user2->id, $member2->id); $this->assertEquals(fullname($user2), $member2->fullname); @@ -5244,7 +5244,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(true, $member2->showonlinestatus); $this->assertEquals(true, $member2->iscontact); $this->assertEquals(false, $member2->isblocked); - $this->assertCount(2, $member2->contactrequests); + $this->assertCount(1, $member2->contactrequests); $this->assertEquals($user3->id, $member3->id); $this->assertEquals(fullname($user3), $member3->fullname); @@ -5252,12 +5252,11 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(true, $member3->showonlinestatus); $this->assertEquals(false, $member3->iscontact); $this->assertEquals(true, $member3->isblocked); - $this->assertCount(2, $member3->contactrequests); + $this->assertCount(1, $member3->contactrequests); // Confirm the contact requests are OK. $request1 = array_shift($member1->contactrequests); $request2 = array_shift($member1->contactrequests); - $request3 = array_shift($member1->contactrequests); $this->assertEquals($user1->id, $request1->userid); $this->assertEquals($user2->id, $request1->requesteduserid); @@ -5265,26 +5264,15 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($user1->id, $request2->userid); $this->assertEquals($user3->id, $request2->requesteduserid); - $this->assertEquals($user1->id, $request3->userid); - $this->assertEquals($user4->id, $request3->requesteduserid); - $request1 = array_shift($member2->contactrequests); - $request2 = array_shift($member2->contactrequests); $this->assertEquals($user1->id, $request1->userid); $this->assertEquals($user2->id, $request1->requesteduserid); - $this->assertEquals($user2->id, $request2->userid); - $this->assertEquals($user3->id, $request2->requesteduserid); - $request1 = array_shift($member3->contactrequests); - $request2 = array_shift($member3->contactrequests); $this->assertEquals($user1->id, $request1->userid); $this->assertEquals($user3->id, $request1->requesteduserid); - - $this->assertEquals($user2->id, $request2->userid); - $this->assertEquals($user3->id, $request2->requesteduserid); } /** From 82e0973c965a7504642906dd4291e8a4cc4367bc Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Fri, 2 Nov 2018 16:17:49 +0800 Subject: [PATCH 4/5] MDL-63850 core_message: helper now returns privacy information --- message/classes/helper.php | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/message/classes/helper.php b/message/classes/helper.php index 407c59b807e..36dcdc92acf 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -483,11 +483,14 @@ class helper { * @param int $referenceuserid the id of the user which check contact and blocked status. * @param array $userids * @param bool $includecontactrequests Do we want to include contact requests with this data? + * @param bool $includeprivacyinfo Do we want to include whether the user can message another, and if the user + * requires a contact. * @return array the array of objects containing member info, indexed by userid. * @throws \coding_exception * @throws \dml_exception */ - public static function get_member_info(int $referenceuserid, array $userids, bool $includecontactrequests = false) : array { + public static function get_member_info(int $referenceuserid, array $userids, bool $includecontactrequests = false, + bool $includeprivacyinfo = false) : array { global $DB, $PAGE; // Prevent exception being thrown when array is empty. @@ -531,6 +534,21 @@ class helper { $data->isdeleted = ($member->deleted) ? true : false; + $data->requirescontact = null; + $data->canmessage = null; + if ($includeprivacyinfo) { + $privacysetting = api::get_user_privacy_messaging_preference($member->id); + $data->requirescontact = $privacysetting == api::MESSAGE_PRIVACY_ONLYCONTACTS; + + $recipient = new \stdClass(); + $recipient->id = $member->id; + + $sender = new \stdClass(); + $sender->id = $referenceuserid; + + $data->canmessage = api::can_post_message($recipient, $sender); + } + $members[$data->id] = $data; } From cef1d977c363188aa4a5d1072ca41c43246d85b6 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 5 Nov 2018 11:51:27 +0800 Subject: [PATCH 5/5] MDL-63850 core_message: get_conversations returns privacy and requests --- message/classes/api.php | 35 ++++++++++---- message/classes/helper.php | 3 ++ message/externallib.php | 4 +- message/tests/api_test.php | 71 ++++++++++++++++++++++++++-- message/tests/externallib_test.php | 74 ++++++++++++++++++++++++++++-- 5 files changed, 171 insertions(+), 16 deletions(-) diff --git a/message/classes/api.php b/message/classes/api.php index cc9ca69e829..cd7e3cc0966 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -558,10 +558,11 @@ class api { $conversationset = $DB->get_recordset_sql($sql, $params, $limitfrom, $limitnum); $conversations = []; - $uniquemembers = []; $members = []; + $individualmembers = []; + $groupmembers = []; foreach ($conversationset as $conversation) { - $conversations[] = $conversation; + $conversations[$conversation->id] = $conversation; $members[$conversation->id] = []; } $conversationset->close(); @@ -597,7 +598,7 @@ class api { 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; + $individualmembers[$conversation->useridfrom] = $conversation->useridfrom; } else { $individualconversations[] = $conversation->id; } @@ -605,7 +606,7 @@ class api { // 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; + $groupmembers[$conversation->useridfrom] = $conversation->useridfrom; } } } @@ -623,10 +624,9 @@ class api { foreach ($conversationmembers as $mid => $member) { $members[$member->conversationid][$member->userid] = $member->userid; - $uniquemembers[$member->userid] = $member->userid; + $individualmembers[$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) @@ -636,8 +636,17 @@ class api { // 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). - if (!empty($memberids)) { - $memberinfo = helper::get_member_info($userid, $memberids); + if (!empty($individualmembers) || !empty($groupmembers)) { + // Now, we want to remove any duplicates from the group members array. For individual members we will + // be doing a more extensive call as we want their contact requests as well as privacy information, + // which is not necessary for group conversations. + $diffgroupmembers = array_diff($groupmembers, $individualmembers); + + $individualmemberinfo = helper::get_member_info($userid, $individualmembers, true, true); + $groupmemberinfo = helper::get_member_info($userid, $diffgroupmembers); + + // Don't use array_merge, as we lose array keys. + $memberinfo = $individualmemberinfo + $groupmemberinfo; // Update the members array with the member information. $deletedmembers = []; @@ -648,7 +657,15 @@ class api { if ($memberinfo[$memberid]->isdeleted) { $deletedmembers[$convid][] = $memberid; } - $members[$convid][$key] = $memberinfo[$memberid]; + + $members[$convid][$key] = clone $memberinfo[$memberid]; + + if ($conversations[$convid]->conversationtype == self::MESSAGE_CONVERSATION_TYPE_GROUP) { + // Remove data we don't need for group. + $members[$convid][$key]->requirescontact = null; + $members[$convid][$key]->canmessage = null; + $members[$convid][$key]->contactrequests = []; + } } } } diff --git a/message/classes/helper.php b/message/classes/helper.php index 36dcdc92acf..8426e29b8ef 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -549,6 +549,9 @@ class helper { $data->canmessage = api::can_post_message($recipient, $sender); } + // Populate the contact requests, even if we don't need them. + $data->contactrequests = []; + $members[$data->id] = $data; } diff --git a/message/externallib.php b/message/externallib.php index af3740a426d..8a126bb26ca 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -964,7 +964,7 @@ class core_message_external extends external_api { '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() + self::get_conversation_member_structure(true) ), 'messages' => new external_multiple_structure( self::get_conversation_message_structure() @@ -992,6 +992,8 @@ class core_message_external extends external_api { 'showonlinestatus' => new external_value(PARAM_BOOL, 'Show the user\'s online status?'), 'isblocked' => new external_value(PARAM_BOOL, 'If the user has been blocked'), 'iscontact' => new external_value(PARAM_BOOL, 'Is the user a contact?'), + 'canmessage' => new external_value(PARAM_BOOL, 'If the user can be messaged'), + 'requirescontact' => new external_value(PARAM_BOOL, 'If the user requires to be contacts'), ]; if ($includecontactrequests) { diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 4e7e518295c..50c3c1fe313 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -1089,6 +1089,9 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertObjectHasAttribute('showonlinestatus', $member); $this->assertObjectHasAttribute('isblocked', $member); $this->assertObjectHasAttribute('iscontact', $member); + $this->assertObjectHasAttribute('canmessage', $member); + $this->assertObjectHasAttribute('requirescontact', $member); + $this->assertObjectHasAttribute('contactrequests', $member); } $this->assertObjectHasAttribute('messages', $conv); foreach ($conv->messages as $message) { @@ -1322,6 +1325,65 @@ class core_message_api_testcase extends core_message_messagelib_testcase { } } + /** + * Test verifying get_conversations when there are users in a group and/or individual conversation. The reason this + * test is performed is because we do not need as much data for group conversations (saving DB calls), so we want + * to confirm this happens. + */ + public function test_get_conversations_user_in_group_and_individual_chat() { + $this->resetAfterTest(); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $user1->id, + $user2->id + ], + 'Individual conversation' + ); + + testhelper::send_fake_message_to_conversation($user1, $conversation->id); + + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [ + $user1->id, + $user2->id, + ], + 'Group conversation' + ); + + testhelper::send_fake_message_to_conversation($user1, $conversation->id); + + \core_message\api::create_contact_request($user1->id, $user2->id); + \core_message\api::create_contact_request($user1->id, $user3->id); + + $conversations = \core_message\api::get_conversations($user2->id); + + $groupconversation = array_shift($conversations); + $individualconversation = array_shift($conversations); + + $this->assertEquals('Group conversation', $groupconversation->name); + $this->assertEquals('Individual conversation', $individualconversation->name); + + $this->assertCount(1, $groupconversation->members); + $this->assertCount(1, $individualconversation->members); + + $groupmember = reset($groupconversation->members); + $this->assertNull($groupmember->requirescontact); + $this->assertNull($groupmember->canmessage); + $this->assertEmpty($groupmember->contactrequests); + + $individualmember = reset($individualconversation->members); + $this->assertNotNull($individualmember->requirescontact); + $this->assertNotNull($individualmember->canmessage); + $this->assertNotEmpty($individualmember->contactrequests); + } + /** * Test verifying that group linked conversations are returned and contain a subname matching the course name. */ @@ -4432,7 +4494,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals(true, $member1->showonlinestatus); $this->assertEquals(false, $member1->iscontact); $this->assertEquals(false, $member1->isblocked); - $this->assertObjectNotHasAttribute('contactrequests', $member1); + $this->assertObjectHasAttribute('contactrequests', $member1); + $this->assertEmpty($member1->contactrequests); $this->assertEquals($user2->id, $member2->id); $this->assertEquals(fullname($user2), $member2->fullname); @@ -4440,7 +4503,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals(true, $member2->showonlinestatus); $this->assertEquals(true, $member2->iscontact); $this->assertEquals(false, $member2->isblocked); - $this->assertObjectNotHasAttribute('contactrequests', $member2); + $this->assertObjectHasAttribute('contactrequests', $member2); + $this->assertEmpty($member2->contactrequests); $this->assertEquals($user3->id, $member3->id); $this->assertEquals(fullname($user3), $member3->fullname); @@ -4448,7 +4512,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals(true, $member3->showonlinestatus); $this->assertEquals(false, $member3->iscontact); $this->assertEquals(true, $member3->isblocked); - $this->assertObjectNotHasAttribute('contactrequests', $member3); + $this->assertObjectHasAttribute('contactrequests', $member3); + $this->assertEmpty($member3->contactrequests); } /** diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index 62ccac4ea99..8c9678ce0ae 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -4781,6 +4781,9 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertArrayHasKey('showonlinestatus', $member); $this->assertArrayHasKey('isblocked', $member); $this->assertArrayHasKey('iscontact', $member); + $this->assertArrayHasKey('canmessage', $member); + $this->assertArrayHasKey('requirescontact', $member); + $this->assertArrayHasKey('contactrequests', $member); } $this->assertArrayHasKey('messages', $conv); foreach ($conv['messages'] as $message) { @@ -5076,6 +5079,68 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($groupimageurl, $conversations[0]['imageurl']); } + /** + * Test verifying get_conversations when there are users in a group and/or individual conversation. The reason this + * test is performed is because we do not need as much data for group conversations (saving DB calls), so we want + * to confirm this happens. + */ + public function test_get_conversations_user_in_group_and_individual_chat() { + $this->resetAfterTest(); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $user1->id, + $user2->id + ], + 'Individual conversation' + ); + + testhelper::send_fake_message_to_conversation($user1, $conversation->id); + + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [ + $user1->id, + $user2->id, + ], + 'Group conversation' + ); + + testhelper::send_fake_message_to_conversation($user1, $conversation->id); + + \core_message\api::create_contact_request($user1->id, $user2->id); + \core_message\api::create_contact_request($user1->id, $user3->id); + + $this->setUser($user2); + $result = core_message_external::get_conversations($user2->id); + $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result); + $conversations = $result['conversations']; + + $groupconversation = array_shift($conversations); + $individualconversation = array_shift($conversations); + + $this->assertEquals('Group conversation', $groupconversation['name']); + $this->assertEquals('Individual conversation', $individualconversation['name']); + + $this->assertCount(1, $groupconversation['members']); + $this->assertCount(1, $individualconversation['members']); + + $groupmember = reset($groupconversation['members']); + $this->assertNull($groupmember['requirescontact']); + $this->assertNull($groupmember['canmessage']); + $this->assertEmpty($groupmember['contactrequests']); + + $individualmember = reset($individualconversation['members']); + $this->assertNotNull($individualmember['requirescontact']); + $this->assertNotNull($individualmember['canmessage']); + $this->assertNotEmpty($individualmember['contactrequests']); + } + /** * Test returning members in a conversation with no contact requests. */ @@ -5161,7 +5226,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(true, $member1->showonlinestatus); $this->assertEquals(false, $member1->iscontact); $this->assertEquals(false, $member1->isblocked); - $this->assertObjectNotHasAttribute('contactrequests', $member1); + $this->assertObjectHasAttribute('contactrequests', $member1); + $this->assertEmpty($member1->contactrequests); $this->assertEquals($user2->id, $member2->id); $this->assertEquals(fullname($user2), $member2->fullname); @@ -5169,7 +5235,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(true, $member2->showonlinestatus); $this->assertEquals(true, $member2->iscontact); $this->assertEquals(false, $member2->isblocked); - $this->assertObjectNotHasAttribute('contactrequests', $member2); + $this->assertObjectHasAttribute('contactrequests', $member2); + $this->assertEmpty($member2->contactrequests); $this->assertEquals($user3->id, $member3->id); $this->assertEquals(fullname($user3), $member3->fullname); @@ -5177,7 +5244,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(true, $member3->showonlinestatus); $this->assertEquals(false, $member3->iscontact); $this->assertEquals(true, $member3->isblocked); - $this->assertObjectNotHasAttribute('contactrequests', $member3); + $this->assertObjectHasAttribute('contactrequests', $member3); + $this->assertEmpty($member3->contactrequests); } /**