L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-63850_master' of git://github.com/markn86/moodle

Browse Source
This commit is contained in:
Andrew Nicols
2018-11-12 07:53:09 +08:00
parent 208950cff0 cef1d977c3
commit e2a8ebddb5
5 changed files with 223 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
message/classes/api.php
+27 -28
View File
@@ -561,10 +561,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();
@@ -600,7 +601,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;
}
@@ -608,7 +609,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;
}
}
}
@@ -626,10 +627,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)
@@ -639,8 +639,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 = [];
@@ -651,7 +660,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 = [];
}
}
}
}
@@ -2490,25 +2507,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;
}
message/classes/helper.php
+48 -1
View File
@@ -482,13 +482,22 @@ 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) : 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.
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
@@ -525,8 +534,46 @@ 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);
}
// Populate the contact requests, even if we don't need them.
$data->contactrequests = [];
$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 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;
}
if (isset($members[$contactrequest->requesteduserid])) {
$members[$contactrequest->requesteduserid]->contactrequests[] = $contactrequest;
}
}
}
}
return $members;
}
message/externallib.php
+3 -1
View File
@@ -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) {
message/tests/api_test.php
+71 -18
View File
@@ -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.
*/
@@ -4450,7 +4512,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);
@@ -4458,7 +4521,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);
@@ -4466,7 +4530,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);
}
/**
@@ -4521,7 +4586,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);
@@ -4529,7 +4594,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);
@@ -4537,12 +4602,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);
@@ -4550,26 +4614,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);
}
/**
message/tests/externallib_test.php
+74 -18
View File
@@ -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);
}
/**
@@ -5236,7 +5304,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 +5312,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 +5320,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 +5332,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);
}
/**