From 9cef5491fb4c5e18c28f4d4a3b06ee90b538973a Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Fri, 9 Nov 2018 17:27:04 +0800 Subject: [PATCH 1/2] MDL-63884 core_message: fix for groups without images If we don't have an image for the group, leave this extra field empty. --- message/classes/api.php | 5 ++++- message/tests/api_test.php | 20 +++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/message/classes/api.php b/message/classes/api.php index 62995e31415..43fa325726f 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -450,7 +450,10 @@ class api { $extrafields[$convid]['subname'] = format_string($courseinfo[$groupid]->courseshortname); // Imageurl. - $extrafields[$convid]['imageurl'] = get_group_picture_url($group, $group->courseid, true)->out(false); + $extrafields[$convid]['imageurl'] = ''; + if ($url = get_group_picture_url($group, $group->courseid, true)) { + $extrafields[$convid]['imageurl'] = $url->out(false); + } } } } diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 76f27779ac8..dceda6f79cb 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -1335,7 +1335,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $course1 = $this->getDataGenerator()->create_course(); - // Create a group with a linked conversation. + // Create a group with a linked conversation and a valid image. $this->setAdminUser(); $this->getDataGenerator()->enrol_user($user1->id, $course1->id); $this->getDataGenerator()->enrol_user($user2->id, $course1->id); @@ -1350,11 +1350,29 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->getDataGenerator()->create_group_member(array('groupid' => $group1->id, 'userid' => $user1->id)); $this->getDataGenerator()->create_group_member(array('groupid' => $group1->id, 'userid' => $user2->id)); + // Verify the group with the image works as expected. $conversations = \core_message\api::get_conversations($user1->id); $this->assertEquals(2, $conversations[0]->membercount); $this->assertEquals($course1->shortname, $conversations[0]->subname); $groupimageurl = get_group_picture_url($group1, $group1->courseid, true); $this->assertEquals($groupimageurl, $conversations[0]->imageurl); + + // Create a group with a linked conversation and without any image. + $group2 = $this->getDataGenerator()->create_group([ + 'courseid' => $course1->id, + 'enablemessaging' => 1, + ]); + + // Add users to group2. + $this->getDataGenerator()->create_group_member(array('groupid' => $group2->id, 'userid' => $user2->id)); + $this->getDataGenerator()->create_group_member(array('groupid' => $group2->id, 'userid' => $user3->id)); + + // Verify the group without any image works as expected too. + $conversations = \core_message\api::get_conversations($user3->id); + $this->assertEquals(2, $conversations[0]->membercount); + $this->assertEquals($course1->shortname, $conversations[0]->subname); + $groupimageurl = get_group_picture_url($group2, $group2->courseid, true); + $this->assertEquals($groupimageurl, $conversations[0]->imageurl); } /** From d76029110bc7ce426b3c8219cf6532b64fc640c4 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Fri, 9 Nov 2018 17:27:55 +0800 Subject: [PATCH 2/2] MDL-63884 core_message: fix for get_conversations legacy adapter code This code also tries to adapt other conversation types, but it should only support individual conversations, where we know members exist. --- message/classes/helper.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/message/classes/helper.php b/message/classes/helper.php index 1927b643ee2..732268434ab 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -540,6 +540,10 @@ class helper { // Transform new data format back into the old format, just for BC during the deprecation life cycle. $tmp = []; foreach ($conversations as $id => $conv) { + // Only individual conversations were supported in legacy messaging. + if ($conv->type != \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL) { + continue; + } $data = new \stdClass(); // The logic for the 'other user' is as follows: // If a conversation is of type 'individual', the other user is always the member who is not the current user.