MDL-64167 core_message: fix counts to exclude 'self' conversations
authorJake Dallimore <jake@moodle.com>
Fri, 23 Nov 2018 05:45:09 +0000 (13:45 +0800)
committerJake Dallimore <jake@moodle.com>
Mon, 26 Nov 2018 00:52:55 +0000 (08:52 +0800)
message/classes/api.php
message/tests/api_test.php
message/tests/externallib_test.php

index b4422be..19a2fa6 100644 (file)
@@ -1523,6 +1523,7 @@ class api {
         // Some restrictions we need to be aware of:
         // - Individual conversations containing soft-deleted user must be counted.
         // - Individual conversations containing only deleted messages must NOT be counted.
+        // - Individual conversations which are legacy 'self' conversations (2 members, both the same user) must NOT be counted.
         // - Group conversations with 0 messages must be counted.
         // - Linked conversations which are disabled (enabled = 0) must NOT be counted.
         // - Any type of conversation can be included in the favourites count, however, the type counts and the favourites count
@@ -1538,6 +1539,17 @@ class api {
                   FROM {message_conversations} mc
             INNER JOIN {message_conversation_members} mcm
                     ON mcm.conversationid = mc.id
+            INNER JOIN (
+                              SELECT mcm.conversationid, count(distinct mcm.userid) as membercount
+                                FROM {message_conversation_members} mcm
+                               WHERE mcm.conversationid IN (
+                                        SELECT DISTINCT conversationid
+                                          FROM {message_conversation_members} mcm2
+                                         WHERE userid = :userid5
+                                     )
+                            GROUP BY mcm.conversationid
+                       ) uniquemembercount
+                    ON uniquemembercount.conversationid = mc.id
              LEFT JOIN (
                               SELECT m.conversationid as convid, MAX(m.timecreated) as maxtime
                                 FROM {messages} m
@@ -1553,7 +1565,10 @@ class api {
                $favsql
                  WHERE mcm.userid = :userid3
                    AND mc.enabled = :enabled
-                   AND ((mc.type = :individualtype AND maxvisibleconvmessage.convid IS NOT NULL) OR (mc.type = :grouptype))
+                   AND (
+                          (mc.type = :individualtype AND maxvisibleconvmessage.convid IS NOT NULL AND membercount > 1) OR
+                          (mc.type = :grouptype)
+                       )
               GROUP BY mc.type, fav.itemtype
               ORDER BY mc.type ASC";
 
@@ -1562,6 +1577,7 @@ class api {
             'userid2' => $userid,
             'userid3' => $userid,
             'userid4' => $userid,
+            'userid5' => $userid,
             'action' => self::MESSAGE_ACTION_DELETED,
             'enabled' => self::MESSAGE_CONVERSATION_ENABLED,
             'individualtype' => self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
index 65dfb4c..5370fc3 100644 (file)
@@ -5706,7 +5706,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
     public function test_get_conversation_counts_test_cases() {
         $typeindividual = \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL;
         $typegroup = \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP;
-        list($user1, $user2, $user3, $user4, $user5, $user6, $user7) = [0, 1, 2, 3, 4, 5, 6];
+        list($user1, $user2, $user3, $user4, $user5, $user6, $user7, $user8) = [0, 1, 2, 3, 4, 5, 6, 7];
         $conversations = [
             [
                 'type' => $typeindividual,
@@ -5736,6 +5736,13 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
                 'favourites' => [$user6],
                 'enabled' => false
             ],
+            [
+                'type' => $typeindividual,
+                'users' => [$user8, $user8],
+                'messages' => [$user8, $user8],
+                'favourites' => [],
+                'enabled' => null // Individual conversations cannot be disabled.
+            ],
         ];
 
         return [
@@ -5926,6 +5933,17 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
                 ]],
                 'deletedusers' => []
             ],
+            'Conversation with self' => [
+                'conversationConfigs' => $conversations,
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user8],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 0
+                ]],
+                'deletedusers' => []
+            ],
         ];
     }
 
@@ -5950,6 +5968,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
     ) {
         $generator = $this->getDataGenerator();
         $users = [
+            $generator->create_user(),
             $generator->create_user(),
             $generator->create_user(),
             $generator->create_user(),
index cb1959e..fbfeb35 100644 (file)
@@ -6138,7 +6138,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
     public function test_get_conversation_counts_test_cases() {
         $typeindividual = \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL;
         $typegroup = \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP;
-        list($user1, $user2, $user3, $user4, $user5, $user6, $user7) = [0, 1, 2, 3, 4, 5, 6];
+        list($user1, $user2, $user3, $user4, $user5, $user6, $user7, $user8) = [0, 1, 2, 3, 4, 5, 6, 7];
         $conversations = [
             [
                 'type' => $typeindividual,
@@ -6168,6 +6168,13 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
                 'favourites' => [$user6],
                 'enabled' => false
             ],
+            [
+                'type' => $typeindividual,
+                'users' => [$user8, $user8],
+                'messages' => [$user8, $user8],
+                'favourites' => [],
+                'enabled' => null // Individual conversations cannot be disabled.
+            ],
         ];
 
         return [
@@ -6358,6 +6365,17 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
                 ]],
                 'deletedusers' => []
             ],
+            'Conversation with self' => [
+                'conversationConfigs' => $conversations,
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user8],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 0
+                ]],
+                'deletedusers' => []
+            ],
         ];
     }
 
@@ -6383,6 +6401,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
         $this->resetAfterTest();
         $generator = $this->getDataGenerator();
         $users = [
+            $generator->create_user(),
             $generator->create_user(),
             $generator->create_user(),
             $generator->create_user(),