From 9e6734a790db0a3dbccc74b502daa636d2354e69 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 19 Dec 2018 08:46:16 +0800 Subject: [PATCH 1/2] MDL-64412 core_message: get_member_info() respects provided ordering This change makes sure the function returns the member information in the same order as those users provided in the $userids param. --- message/classes/helper.php | 3 ++ message/tests/helper_test.php | 70 +++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 message/tests/helper_test.php diff --git a/message/classes/helper.php b/message/classes/helper.php index 99ecdfabe4c..58dd7aa174e 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -589,6 +589,9 @@ class helper { } } + // Return member information in the same order as the userids originally provided. + $members = array_replace(array_flip($userids), $members); + return $members; } diff --git a/message/tests/helper_test.php b/message/tests/helper_test.php new file mode 100644 index 00000000000..8de40f6493f --- /dev/null +++ b/message/tests/helper_test.php @@ -0,0 +1,70 @@ +. + +/** + * Contains a test class for the message helper. + * + * @package core_message + * @category test + * @copyright 2018 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +require_once($CFG->dirroot . '/message/tests/messagelib_test.php'); + +/** + * Tests for the message helper class. + * + * @package core_message + * @category test + * @copyright 2018 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_message_helper_testcase extends advanced_testcase { + + public function setUp() { + $this->resetAfterTest(true); + } + + public function test_get_member_info_ordering() { + // Create a conversation with several users. + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + $user4 = self::getDataGenerator()->create_user(); + + \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [ + $user1->id, + $user2->id, + $user3->id, + $user4->id, + ], + 'Group conversation' + ); + + // Verify that the member information comes back in the same order that we specified in the input array. + $memberinfo = \core_message\helper::get_member_info($user1->id, [$user3->id, $user4->id, $user2->id]); + $this->assertEquals($user3->id, array_shift($memberinfo)->id); + $this->assertEquals($user4->id, array_shift($memberinfo)->id); + $this->assertEquals($user2->id, array_shift($memberinfo)->id); + } +} From a3a6e3c9053b25d27cc17ed849e82341845cb286 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 19 Dec 2018 09:02:59 +0800 Subject: [PATCH 2/2] MDL-64412 core_message: remove ordering code from message_search_users() This code is no longer needed as get_member_info() now respects the ordering of the $userids param. --- message/classes/api.php | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/message/classes/api.php b/message/classes/api.php index a138fe4ff50..4c389637e5a 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -323,15 +323,11 @@ class api { ORDER BY " . $DB->sql_fullname(); $foundusers = $DB->get_records_sql_menu($sql, $params + $excludeparams, $limitfrom, $limitnum); - $orderedcontacts = array(); + $contacts = []; 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. - // We need to reorder it again based on query's result. - foreach ($foundusers as $key => $value) { - $contact = $contacts[$key]; - $contact->conversations = self::get_conversations_between_users($userid, $key, 0, 1000); - $orderedcontacts[] = $contact; + foreach ($contacts as $memberuserid => $memberinfo) { + $contacts[$memberuserid]->conversations = self::get_conversations_between_users($userid, $memberuserid, 0, 1000); } } @@ -417,19 +413,15 @@ class api { $foundusers = $returnedusers; } - $orderednoncontacts = array(); + $noncontacts = []; 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. - // We need to reorder it again based on query's result. - foreach ($foundusers as $key => $value) { - $contact = $noncontacts[$key]; - $contact->conversations = self::get_conversations_between_users($userid, $key, 0, 1000); - $orderednoncontacts[] = $contact; + foreach ($noncontacts as $memberuserid => $memberinfo) { + $noncontacts[$memberuserid]->conversations = self::get_conversations_between_users($userid, $memberuserid, 0, 1000); } } - return array($orderedcontacts, $orderednoncontacts); + return array(array_values($contacts), array_values($noncontacts)); } /**