diff --git a/message/classes/api.php b/message/classes/api.php index 9d6427a71ba..163150dff45 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -516,12 +516,10 @@ class api { // If we need to return ONLY favourites, or NO favourites, generate the SQL snippet. $favouritesql = ""; $favouriteparams = []; - if (is_bool($favourites)) { - if (!empty($favouriteconversationids)) { - list ($insql, $inparams) = $DB->get_in_or_equal($favouriteconversationids, SQL_PARAMS_NAMED, 'favouriteids'); - $favouritesql = $favourites ? " AND mc.id {$insql} " : " AND mc.id NOT {$insql} "; - $favouriteparams = $inparams; - } + if (null !== $favourites && !empty($favouriteconversationids)) { + list ($insql, $favouriteparams) = + $DB->get_in_or_equal($favouriteconversationids, SQL_PARAMS_NAMED, 'favouriteids', $favourites); + $favouritesql = " AND mc.id {$insql} "; } // If we need to restrict type, generate the SQL snippet. diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 5b5f8da1a93..babfa107017 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -1275,6 +1275,51 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertNotContains($ic1->id, array_column($conversations, 'id')); } + /** + * Test verifying the behaviour of get_conversations() when fetching favourite conversations with only a single + * favourite. + */ + public function test_get_conversations_favourite_conversations_single() { + // Get a bunch of conversations, some group, some individual and in different states. + list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3, + $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data(); + + // Mark a single conversation as favourites. + \core_message\api::set_favourite_conversation($ic2->id, $user1->id); + + // Get the conversation, first with no restrictions, confirming the favourite status of the conversations. + $conversations = \core_message\api::get_conversations($user1->id); + $this->assertCount(6, $conversations); + foreach ($conversations as $conv) { + if (in_array($conv->id, [$ic2->id])) { + $this->assertTrue($conv->isfavourite); + } else { + $this->assertFalse($conv->isfavourite); + } + } + + // Now, get ONLY favourite conversations. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, null, true); + $this->assertCount(1, $conversations); + foreach ($conversations as $conv) { + $this->assertTrue($conv->isfavourite); + $this->assertEquals($ic2->id, $conv->id); + } + + // Now, try ONLY favourites of type 'group'. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, true); + $this->assertEmpty($conversations); + + // And NO favourite conversations. + $conversations = \core_message\api::get_conversations($user1->id, 0, 20, null, false); + $this->assertCount(5, $conversations); + foreach ($conversations as $conv) { + $this->assertFalse($conv->isfavourite); + $this->assertNotEquals($ic2, $conv->id); + } + } + /** * Test verifying the behaviour of get_conversations() when fetching favourite conversations. */ @@ -1293,13 +1338,16 @@ class core_message_api_testcase extends core_message_messagelib_testcase { \core_message\api::set_favourite_conversation($ic1->id, $user1->id); \core_message\api::set_favourite_conversation($gc2->id, $user1->id); \core_message\api::set_favourite_conversation($gc5->id, $user1->id); + $favouriteids = [$ic1->id, $gc2->id, $gc5->id]; // Get the conversations, first with no restrictions, confirming the favourite status of the conversations. $conversations = \core_message\api::get_conversations($user1->id); $this->assertCount(6, $conversations); foreach ($conversations as $conv) { - if (in_array($conv->id, [$ic1->id, $gc2->id, $gc5->id])) { + if (in_array($conv->id, $favouriteids)) { $this->assertTrue($conv->isfavourite); + } else { + $this->assertFalse($conv->isfavourite); } } @@ -1308,6 +1356,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertCount(3, $conversations); foreach ($conversations as $conv) { $this->assertTrue($conv->isfavourite); + $this->assertNotFalse(array_search($conv->id, $favouriteids)); } // Now, try ONLY favourites of type 'group'. @@ -1316,6 +1365,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertCount(2, $conversations); foreach ($conversations as $conv) { $this->assertTrue($conv->isfavourite); + $this->assertNotFalse(array_search($conv->id, [$gc2->id, $gc5->id])); } // And NO favourite conversations. @@ -1323,6 +1373,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertCount(3, $conversations); foreach ($conversations as $conv) { $this->assertFalse($conv->isfavourite); + $this->assertFalse(array_search($conv->id, $favouriteids)); } }