MDL-64057 core_message: consolidate conversation counts into single call
authorJake Dallimore <jake@moodle.com>
Tue, 20 Nov 2018 05:15:27 +0000 (13:15 +0800)
committerJake Dallimore <jake@moodle.com>
Fri, 23 Nov 2018 03:02:19 +0000 (11:02 +0800)
Counts for types (individual, group) and favourites can be fetched with
a single API call.

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

index 86a22f8..415f2ae 100644 (file)
@@ -1459,78 +1459,80 @@ class api {
      * Returns the count of conversations (collection of messages from a single user) for
      * the given user.
      *
-     * @param \stdClass $user The user who's conversations should be counted
-     * @param int $type The conversation type
-     * @param bool $excludefavourites Exclude favourite conversations
-     * @return int the count of the user's unread conversations
+     * @param int $userid The user whose conversations should be counted.
+     * @return array the array of conversations counts, indexed by type.
      */
-    public static function count_conversations($user, int $type = null, bool $excludefavourites = false) {
+    public static function get_conversation_counts(int $userid) : array {
         global $DB;
 
-        $params = [];
-        $favouritessql = '';
-
-        if ($excludefavourites) {
-            $favouritessql = "AND m.conversationid NOT IN (
-                                SELECT itemid
-                                FROM {favourite}
-                                WHERE component = 'core_message'
-                                AND itemtype = 'message_conversations'
-                                AND userid = ?
-                            )";
-            $params[] = $user->id;
-        }
-
-        switch($type) {
-            case null:
-                $params = array_merge([$user->id, self::MESSAGE_ACTION_DELETED, $user->id], $params);
-                $sql = "SELECT COUNT(DISTINCT(m.conversationid))
-                          FROM {messages} m
-                     LEFT JOIN {message_conversations} c
-                            ON m.conversationid = c.id
-                     LEFT JOIN {message_user_actions} ma
-                            ON ma.messageid = m.id
-                     LEFT JOIN {message_conversation_members} mcm
-                            ON m.conversationid = mcm.conversationid
-                         WHERE mcm.userid = ?
-                           AND (
-                                    c.type != " . self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL . "
-                                    OR
-                                    (
-                                        (ma.action IS NULL OR ma.action != ? OR ma.userid != ?)
-                                        AND c.type = " . self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL . "
-                                    )
-                                )
-                               ${favouritessql}";
-                break;
-            case self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL:
-                $params = array_merge([self::MESSAGE_ACTION_DELETED, $user->id, $user->id], $params);
-                $sql = "SELECT COUNT(DISTINCT(m.conversationid))
-                          FROM {messages} m
-                     LEFT JOIN {message_conversations} c
-                            ON m.conversationid = c.id
-                     LEFT JOIN {message_user_actions} ma
-                            ON ma.messageid = m.id
-                     LEFT JOIN {message_conversation_members} mcm
-                            ON m.conversationid = mcm.conversationid
-                         WHERE (ma.action IS NULL OR ma.action != ? OR ma.userid != ?)
-                           AND mcm.userid = ?
-                           AND c.type = " . self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL . "
-                               ${favouritessql}";
-                break;
-            default:
-                $params = array_merge([$user->id, $type], $params);
-                $sql = "SELECT COUNT(m.conversationid)
-                          FROM {message_conversation_members} m
-                     LEFT JOIN {message_conversations} c
-                            ON m.conversationid = c.id
-                         WHERE m.userid = ?
-                           AND c.type = ?
-                               ${favouritessql}";
-
-        }
-
-        return $DB->count_records_sql($sql, $params);
+        // 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.
+        // - 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
+        // are mutually exclusive; any conversations which are counted in favourites cannot be counted elsewhere.
+
+        // First, ask the favourites service to give us the join SQL for favourited conversations,
+        // so we can include favourite information in the query.
+        $usercontext = \context_user::instance($userid);
+        $favservice = \core_favourites\service_factory::get_service_for_user_context($usercontext);
+        list($favsql, $favparams) = $favservice->get_join_sql_by_type('core_message', 'message_conversations', 'fav', 'mc.id');
+
+        $sql = "SELECT mc.type, fav.itemtype, COUNT(DISTINCT mc.id) as count
+                  FROM {message_conversations} mc
+            INNER JOIN {message_conversation_members} mcm
+                    ON mcm.conversationid = mc.id
+             LEFT JOIN (
+                              SELECT m.conversationid as convid, MAX(m.timecreated) as maxtime
+                                FROM {messages} m
+                          INNER JOIN {message_conversation_members} mcm
+                                  ON mcm.conversationid = m.conversationid
+                           LEFT JOIN {message_user_actions} mua
+                                  ON (mua.messageid = m.id AND mua.userid = :userid AND mua.action = :action)
+                               WHERE mua.id is NULL
+                                 AND mcm.userid = :userid2
+                            GROUP BY m.conversationid
+                       ) maxvisibleconvmessage
+                    ON maxvisibleconvmessage.convid = mc.id
+               $favsql
+                 WHERE mcm.userid = :userid3
+                   AND mc.enabled = :enabled
+                   AND ((mc.type = :individualtype AND maxvisibleconvmessage.convid IS NOT NULL) OR (mc.type = :grouptype))
+              GROUP BY mc.type, fav.itemtype
+              ORDER BY mc.type ASC";
+
+        $params = [
+            'userid' => $userid,
+            'userid2' => $userid,
+            'userid3' => $userid,
+            'userid4' => $userid,
+            'action' => self::MESSAGE_ACTION_DELETED,
+            'enabled' => self::MESSAGE_CONVERSATION_ENABLED,
+            'individualtype' => self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
+            'grouptype' => self::MESSAGE_CONVERSATION_TYPE_GROUP,
+        ] + $favparams;
+
+        // Assemble the return array.
+        $counts = [
+            'favourites' => 0,
+            'types' => [
+                self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                self::MESSAGE_CONVERSATION_TYPE_GROUP => 0
+            ]
+        ];
+
+        $countsrs = $DB->get_recordset_sql($sql, $params);
+        foreach ($countsrs as $key => $val) {
+            if (!empty($val->itemtype)) {
+                $counts['favourites'] = $val->count;
+                continue;
+            }
+            $counts['types'][$val->type] = $val->count;
+        }
+        $countsrs->close();
+
+        return $counts;
     }
 
     /**
index 9e29bd1..90167d3 100644 (file)
@@ -5613,196 +5613,257 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
     }
 
     /**
-     * Data provider for test_count_conversations().
+     * Data provider for test_get_conversation_counts().
      */
-    public function test_count_conversations_test_cases() {
+    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) = [0, 1, 2, 3, 4];
+        list($user1, $user2, $user3, $user4, $user5, $user6, $user7) = [0, 1, 2, 3, 4, 5, 6];
         $conversations = [
             [
                 'type' => $typeindividual,
                 'users' => [$user1, $user2],
                 'messages' => [$user1, $user2],
-                'favourites' => [$user1]
+                'favourites' => [$user1],
+                'enabled' => null // Individual conversations cannot be disabled.
             ],
             [
                 'type' => $typeindividual,
                 'users' => [$user1, $user3],
                 'messages' => [$user1, $user1],
-                'favourites' => []
+                'favourites' => [],
+                'enabled' => null // Individual conversations cannot be disabled.
             ],
             [
                 'type' => $typegroup,
                 'users' => [$user1, $user2, $user3, $user4],
                 'messages' => [$user1, $user2, $user3, $user4],
-                'favourites' => []
+                'favourites' => [],
+                'enabled' => true
+            ],
+            [
+                'type' => $typegroup,
+                'users' => [$user6, $user7],
+                'messages' => [$user6, $user7],
+                'favourites' => [$user6],
+                'enabled' => false
             ],
         ];
 
         return [
             'No conversations' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
                 'arguments' => [$user5],
-                'expected' => 0
-            ],
-            'No individual conversations' => [
-                'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user4, $typeindividual],
-                'expected' => 0
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 0
+                ]],
+                'deletedusers' => []
             ],
             'No individual conversations, 1 group conversation' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
                 'arguments' => [$user4],
-                'expected' => 1
-            ],
-            '1 - Multiple individual conversations, 1 group conversation' => [
-                'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user1, $typegroup],
-                'expected' => 1
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            '2 - Multiple individual conversations, 1 group conversation' => [
+            '2 individual conversations (one favourited), 1 group conversation' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user2, $typegroup],
-                'expected' => 1
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 1, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            '3 - Multiple individual conversations, 1 group conversation' => [
+            '1 individual conversation, 1 group conversation' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user3, $typegroup],
-                'expected' => 1
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user2],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            '4 - Multiple individual conversations, 1 group conversation' => [
+            '1 group conversation only' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user4, $typegroup],
-                'expected' => 1
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user4],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'Individual exclude favourites' => [
+            'All conversation types, delete a message from individual favourited, messages remaining' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user1, $typeindividual, true],
-                'expected' => 1
+                'deletemessagesuser' => $user1,
+                'deletemessages' => [0],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 1, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'Individual include favourites' => [
+            'All conversation types, delete a message from individual non-favourited, messages remaining' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user1, $typeindividual, false],
-                'expected' => 2
+                'deletemessagesuser' => $user1,
+                'deletemessages' => [2],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 1, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'All exclude favourites' => [
+            'All conversation types, delete all messages from individual favourited, no messages remaining' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user1, null, true],
-                'expected' => 2
+                'deletemessagesuser' => $user1,
+                'deletemessages' => [0, 1],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'All include favourites' => [
+            'All conversation types, delete all messages from individual non-favourited, no messages remaining' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => null,
-                'delete' => [],
-                'arguments' => [$user1, null, false],
-                'expected' => 3
+                'deletemessagesuser' => $user1,
+                'deletemessages' => [2, 3],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 1, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'Delete single message individual' => [
+            'All conversation types, delete all messages from individual favourited, no messages remaining, different user' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [1],
-                'arguments' => [$user1, $typeindividual],
-                'expected' => 2
+                'deletemessagesuser' => $user1,
+                'deletemessages' => [0, 1],
+                'arguments' => [$user2],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'Delete single message all' => [
+            'All conversation types, delete all messages from individual non-favourited, no messages remaining, different user' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [1],
-                'arguments' => [$user1, null],
-                'expected' => 3
+                'deletemessagesuser' => $user1,
+                'deletemessages' => [2, 3],
+                'arguments' => [$user3],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'Delete all message individual conversation include favourites' => [
+            'All conversation types, delete some messages from group non-favourited, messages remaining,' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [2, 3],
-                'arguments' => [$user1, $typeindividual, false],
-                'expected' => 1
+                'deletemessagesuser' => $user1,
+                'deletemessages' => [4, 5],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 1, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'Delete all message individual conversation exclude favourites' => [
+            'All conversation types, delete all messages from group non-favourited, no messages remaining,' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [2, 3],
-                'arguments' => [$user1, $typeindividual, true],
-                'expected' => 0
+                'deletemessagesuser' => $user1,
+                'deletemessages' => [4, 5, 6, 7],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 1, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => []
             ],
-            'Delete all message individual conversation include favourites diff user' => [
+            'All conversation types, another user soft deleted' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [2, 3],
-                'arguments' => [$user2, $typeindividual, false],
-                'expected' => 1
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 1, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => [$user2]
             ],
-            'Delete all message individual conversation exclude favourites diff user' => [
+            'All conversation types, all group users soft deleted' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [2, 3],
-                'arguments' => [$user2, $typeindividual, true],
-                'expected' => 1
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user1],
+                'expected' => ['favourites' => 1, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 1,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 1
+                ]],
+                'deletedusers' => [$user2, $user3, $user4]
             ],
-            'Delete all message group conversation include favourites' => [
+            'Group conversation which is disabled, favourited' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [4, 5, 6, 7],
-                'arguments' => [$user1, $typegroup, false],
-                'expected' => 1
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user6],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 0
+                ]],
+                'deletedusers' => []
             ],
-            'Delete all message group conversation include favourites' => [
+            'Group conversation which is disabled, non-favourited' => [
                 'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [4, 5, 6, 7],
-                'arguments' => [$user1, null, false],
-                'expected' => 3
+                'deletemessagesuser' => null,
+                'deletemessages' => [],
+                'arguments' => [$user7],
+                'expected' => ['favourites' => 0, 'types' => [
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL => 0,
+                    \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP => 0
+                ]],
+                'deletedusers' => []
             ],
-            'Delete all message group conversation exclude favourites' => [
-                'conversationConfigs' => $conversations,
-                'deleteuser' => $user1,
-                'delete' => [4, 5, 6, 7],
-                'arguments' => [$user1, null, true],
-                'expected' => 2
-            ]
         ];
     }
 
     /**
-     * Test the count_conversations() function.
+     * Test the get_conversation_counts() function.
      *
-     * @dataProvider test_count_conversations_test_cases()
+     * @dataProvider test_get_conversation_counts_test_cases()
      * @param array $conversationconfigs Conversations to create
-     * @param int $deleteuser The user who is deleting the messages
-     * @param array $delete The list of messages to delete (by index)
+     * @param int $deletemessagesuser The user who is deleting the messages
+     * @param array $deletemessages The list of messages to delete (by index)
      * @param array $arguments Arguments for the count conversations function
-     * @param int $expected The expected result
+     * @param array $expected The expected result
+     * @param array $deletedusers the array of users to soft delete.
      */
-    public function test_count_conversations(
+    public function test_get_conversation_counts(
         $conversationconfigs,
-        $deleteuser,
-        $delete,
+        $deletemessagesuser,
+        $deletemessages,
         $arguments,
-        $expected
+        $expected,
+        $deletedusers
     ) {
         $generator = $this->getDataGenerator();
         $users = [
+            $generator->create_user(),
+            $generator->create_user(),
             $generator->create_user(),
             $generator->create_user(),
             $generator->create_user(),
@@ -5810,9 +5871,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
             $generator->create_user()
         ];
 
-        $user = $users[$arguments[0]];
-        $deleteuser = !is_null($deleteuser) ? $users[$deleteuser] : null;
-        $arguments[0] = $user;
+        $deleteuser = !is_null($deletemessagesuser) ? $users[$deletemessagesuser] : null;
+        $arguments[0] = $users[$arguments[0]]->id;
         $systemcontext = \context_system::instance();
         $conversations = [];
         $messageids = [];
@@ -5822,7 +5882,9 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
                 $config['type'],
                 array_map(function($userindex) use ($users) {
                     return $users[$userindex]->id;
-                }, $config['users'])
+                }, $config['users']),
+                null,
+                ($config['enabled'] ?? true)
             );
 
             foreach ($config['messages'] as $userfromindex) {
@@ -5840,11 +5902,21 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
             $conversations[] = $conversation;
         }
 
-        foreach ($delete as $messageindex) {
+        foreach ($deletemessages as $messageindex) {
             \core_message\api::delete_message($deleteuser->id, $messageids[$messageindex]);
         }
 
-        $this->assertEquals($expected, \core_message\api::count_conversations(...$arguments));
+        foreach ($deletedusers as $deleteduser) {
+            delete_user($users[$deleteduser]);
+        }
+
+        $counts = \core_message\api::get_conversation_counts(...$arguments);
+
+        $this->assertEquals($expected['favourites'], $counts['favourites']);
+        $this->assertEquals($expected['types'][\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL],
+            $counts['types'][\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL]);
+        $this->assertEquals($expected['types'][\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP],
+            $counts['types'][\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP]);
     }
 
     /**