MDL-64057 core_message: return conversations with soft-deleted users
authorJake Dallimore <jake@moodle.com>
Thu, 22 Nov 2018 01:15:05 +0000 (09:15 +0800)
committerJake Dallimore <jake@moodle.com>
Fri, 23 Nov 2018 03:02:20 +0000 (11:02 +0800)
Individual conversations with a soft-deleted user (including messages)
are now displayed in the conversation listing. This brings the behaviour
of individual conversations in line with group conversations, in that
the active user can still view messages.

message/classes/api.php
message/classes/helper.php
message/tests/api_test.php
message/tests/externallib_test.php

index 415f2ae..c42d69b 100644 (file)
@@ -730,12 +730,11 @@ class api {
         // Now, create the final return structure.
         $arrconversations = [];
         foreach ($conversations as $conversation) {
-            // 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).
+            // Do not include any individual conversations which do 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))) {
+                    && (empty($conversation->messageid))) {
                 continue;
             }
 
index 2e9c99a..c51c619 100644 (file)
@@ -557,7 +557,7 @@ class helper {
                 $sender = new \stdClass();
                 $sender->id = $referenceuserid;
 
-                $data->canmessage = api::can_post_message($recipient, $sender);
+                $data->canmessage = !$data->isdeleted && api::can_post_message($recipient, $sender);
             }
 
             // Populate the contact requests, even if we don't need them.
index 90167d3..2ac4b32 100644 (file)
@@ -925,8 +925,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         // Retrieve the conversations.
         $conversations = \core_message\api::get_conversations($user1->id, 0, 20, null, true);
 
-        // We should only have one conversation because the other user was deleted.
-        $this->assertCount(1, $conversations);
+        // We should have both conversations, despite the other user being soft-deleted.
+        $this->assertCount(2, $conversations);
 
         // Confirm the conversation is from the non-deleted user.
         $conversation = reset($conversations);
@@ -1373,27 +1373,28 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
             $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data();
 
         // Delete the second user and retrieve the conversations.
-        // We should have 5, as $ic1 drops off the list.
-        // Group conversations remain albeit with less members.
+        // We should have 6 still, as conversations with soft-deleted users are still returned.
+        // Group conversations are also present, albeit with less members.
         delete_user($user2);
         // This is to confirm an exception is not thrown when a user AND the user context is deleted.
         // We no longer delete the user context, but historically we did.
         context_helper::delete_instance(CONTEXT_USER, $user2->id);
         $conversations = \core_message\api::get_conversations($user1->id);
-        $this->assertCount(5, $conversations);
+        $this->assertCount(6, $conversations);
         $this->assertEquals($gc3->id, $conversations[0]->id);
         $this->assertcount(1, $conversations[0]->members);
         $this->assertEquals($gc2->id, $conversations[1]->id);
         $this->assertcount(1, $conversations[1]->members);
         $this->assertEquals($ic2->id, $conversations[2]->id);
-        $this->assertEquals($gc5->id, $conversations[3]->id);
-        $this->assertEquals($gc4->id, $conversations[4]->id);
+        $this->assertEquals($ic1->id, $conversations[3]->id);
+        $this->assertEquals($gc5->id, $conversations[4]->id);
+        $this->assertEquals($gc4->id, $conversations[5]->id);
 
         // Delete a user from a group conversation where that user had sent the most recent message.
         // This user will still be present in the members array, as will the message in the messages array.
         delete_user($user4);
         $conversations = \core_message\api::get_conversations($user1->id);
-        $this->assertCount(5, $conversations);
+        $this->assertCount(6, $conversations);
         $this->assertEquals($gc2->id, $conversations[1]->id);
         $this->assertcount(1, $conversations[1]->members);
         $this->assertEquals($user4->id, $conversations[1]->members[$user4->id]->id);
@@ -1401,17 +1402,19 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->assertEquals($user4->id, $conversations[1]->messages[0]->useridfrom);
 
         // Delete the third user and retrieve the conversations.
-        // We should have 4, as $ic1, $ic2 drop off the list.
-        // Group conversations remain albeit with less members.
+        // We should have 6 still, as conversations with soft-deleted users are still returned.
+        // Group conversations are also present, albeit with less members.
         delete_user($user3);
         $conversations = \core_message\api::get_conversations($user1->id);
-        $this->assertCount(4, $conversations);
+        $this->assertCount(6, $conversations);
         $this->assertEquals($gc3->id, $conversations[0]->id);
         $this->assertcount(1, $conversations[0]->members);
         $this->assertEquals($gc2->id, $conversations[1]->id);
         $this->assertcount(1, $conversations[1]->members);
-        $this->assertEquals($gc5->id, $conversations[2]->id);
-        $this->assertEquals($gc4->id, $conversations[3]->id);
+        $this->assertEquals($ic2->id, $conversations[2]->id);
+        $this->assertEquals($ic1->id, $conversations[3]->id);
+        $this->assertEquals($gc5->id, $conversations[4]->id);
+        $this->assertEquals($gc4->id, $conversations[5]->id);
     }
 
     /**
index 01e10d1..b5fc933 100644 (file)
@@ -5026,20 +5026,21 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $this->setUser($user1);
 
         // Delete the second user and retrieve the conversations.
-        // We should have 5, as $ic1 drops off the list.
-        // Group conversations remain albeit with less members.
+        // We should have 6 still, as conversations with soft-deleted users are still returned.
+        // Group conversations are also present, albeit with less members.
         delete_user($user2);
         $result = core_message_external::get_conversations($user1->id);
         $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result);
         $conversations = $result['conversations'];
-        $this->assertCount(5, $conversations);
+        $this->assertCount(6, $conversations);
         $this->assertEquals($gc3->id, $conversations[0]['id']);
         $this->assertcount(1, $conversations[0]['members']);
         $this->assertEquals($gc2->id, $conversations[1]['id']);
         $this->assertcount(1, $conversations[1]['members']);
         $this->assertEquals($ic2->id, $conversations[2]['id']);
-        $this->assertEquals($gc5->id, $conversations[3]['id']);
-        $this->assertEquals($gc4->id, $conversations[4]['id']);
+        $this->assertEquals($ic1->id, $conversations[3]['id']);
+        $this->assertEquals($gc5->id, $conversations[4]['id']);
+        $this->assertEquals($gc4->id, $conversations[5]['id']);
 
         // Delete a user from a group conversation where that user had sent the most recent message.
         // This user will still be present in the members array, as will the message in the messages array.
@@ -5047,7 +5048,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $result = core_message_external::get_conversations($user1->id);
         $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result);
         $conversations = $result['conversations'];
-        $this->assertCount(5, $conversations);
+        $this->assertCount(6, $conversations);
         $this->assertEquals($gc2->id, $conversations[1]['id']);
         $this->assertcount(1, $conversations[1]['members']);
         $this->assertEquals($user4->id, $conversations[1]['members'][0]['id']);
@@ -5055,19 +5056,21 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $this->assertEquals($user4->id, $conversations[1]['messages'][0]['useridfrom']);
 
         // Delete the third user and retrieve the conversations.
-        // We should have 4, as $ic1, $ic2 drop off the list.
-        // Group conversations remain albeit with less members.
+        // We should have 6 still, as conversations with soft-deleted users are still returned.
+        // Group conversations are also present, albeit with less members.
         delete_user($user3);
         $result = core_message_external::get_conversations($user1->id);
         $result = external_api::clean_returnvalue(core_message_external::get_conversations_returns(), $result);
         $conversations = $result['conversations'];
-        $this->assertCount(4, $conversations);
+        $this->assertCount(6, $conversations);
         $this->assertEquals($gc3->id, $conversations[0]['id']);
         $this->assertcount(1, $conversations[0]['members']);
         $this->assertEquals($gc2->id, $conversations[1]['id']);
         $this->assertcount(1, $conversations[1]['members']);
-        $this->assertEquals($gc5->id, $conversations[2]['id']);
-        $this->assertEquals($gc4->id, $conversations[3]['id']);
+        $this->assertEquals($ic2->id, $conversations[2]['id']);
+        $this->assertEquals($ic1->id, $conversations[3]['id']);
+        $this->assertEquals($gc5->id, $conversations[4]['id']);
+        $this->assertEquals($gc4->id, $conversations[5]['id']);
     }
 
     /**