From aa7483516477c5f81d4e27a8d804b6bc6022954a Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Fri, 2 Nov 2018 10:10:37 +0800 Subject: [PATCH] MDL-63549 core_message: exclude conversations where all messages deleted If there are no recent messages for an individual conversation for the user, then we exclude that conversation from the results. --- message/classes/api.php | 11 ++++++----- message/tests/api_test.php | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/message/classes/api.php b/message/classes/api.php index 08243942a69..5e2c9b1d57f 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -543,11 +543,12 @@ class api { // Now, create the final return structure. $arrconversations = []; foreach ($conversations as $conversation) { - // It's possible other users have been deleted. - // In cases like this, we still want to include the conversation if it's of type 'group'. - // Individual conversations are skipped if the other member has been deleted. - if (isset($deletedmembers[$conversation->id]) && - $conversation->conversationtype == self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL) { + // Do not include any individual conversation which: + // a) Contains a deleted member or + // b) Does not contain a recent message for the user (this happens if the user has deleted all messages). + // Group conversations with deleted users or no messages are always returned. + if ($conversation->conversationtype == self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL + && (isset($deletedmembers[$conversation->id]) || empty($conversation->messageid))) { continue; } diff --git a/message/tests/api_test.php b/message/tests/api_test.php index d75284306ab..ade68433760 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -927,6 +927,40 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $this->assertEquals($gc4->id, $conversations[3]->id); } + /** + * Test confirming the behaviour of get_conversations() when users delete all messages. + */ + public function test_get_conversations_deleted_messages() { + // 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(); + + $conversations = \core_message\api::get_conversations($user1->id); + $this->assertCount(6, $conversations); + + // Delete all messages from a group conversation the user is in - it should be returned. + $this->assertTrue(\core_message\api::is_user_in_conversation($user1->id, $gc2->id)); + $convmessages = \core_message\api::get_conversation_messages($user1->id, $gc2->id); + $messages = $convmessages['messages']; + foreach ($messages as $message) { + \core_message\api::delete_message($user1->id, $message->id); + } + $conversations = \core_message\api::get_conversations($user1->id); + $this->assertCount(6, $conversations); + $this->assertContains($gc2->id, array_column($conversations, 'id')); + + // Delete all messages from an individual conversation the user is in - it should not be returned. + $this->assertTrue(\core_message\api::is_user_in_conversation($user1->id, $ic1->id)); + $convmessages = \core_message\api::get_conversation_messages($user1->id, $ic1->id); + $messages = $convmessages['messages']; + foreach ($messages as $message) { + \core_message\api::delete_message($user1->id, $message->id); + } + $conversations = \core_message\api::get_conversations($user1->id); + $this->assertCount(5, $conversations); + $this->assertNotContains($ic1->id, array_column($conversations, 'id')); + } + /** * Test verifying the behaviour of get_conversations() when fetching favourite conversations. */