MDL-36941 core_message: dont query DB unnecessarily checking read status
authorMark Nelson <markn@moodle.com>
Thu, 22 Mar 2018 11:47:12 +0000 (19:47 +0800)
committerMark Nelson <markn@moodle.com>
Fri, 23 Mar 2018 04:30:31 +0000 (12:30 +0800)
message/classes/api.php
message/externallib.php

index 39a4461..25246a0 100644 (file)
@@ -723,8 +723,14 @@ class api {
                            ON mc.id = m.conversationid
                    INNER JOIN {message_conversation_members} mcm
                            ON mcm.conversationid = mc.id
-                        WHERE mcm.userid = ?
+                    LEFT JOIN {message_user_actions} mua
+                           ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ?)
+                        WHERE mua.id is NULL
+                          AND mcm.userid = ?
                           AND m.useridfrom != ?";
+        $messageparams = [];
+        $messageparams[] = $userid;
+        $messageparams[] = self::MESSAGE_ACTION_READ;
         $messageparams[] = $userid;
         $messageparams[] = $userid;
         if (!is_null($conversationid)) {
@@ -1100,35 +1106,31 @@ class api {
             $timeread = time();
         }
 
-        // Check if the user has already read this message.
-        if (!$DB->record_exists('message_user_actions', ['userid' => $userid,
-                'messageid' => $message->id, 'action' => self::MESSAGE_ACTION_READ])) {
-            $mua = new \stdClass();
-            $mua->userid = $userid;
-            $mua->messageid = $message->id;
-            $mua->action = self::MESSAGE_ACTION_READ;
-            $mua->timecreated = $timeread;
-            $mua->id = $DB->insert_record('message_user_actions', $mua);
-
-            // Get the context for the user who received the message.
-            $context = \context_user::instance($userid, IGNORE_MISSING);
-            // If the user no longer exists the context value will be false, in this case use the system context.
-            if ($context === false) {
-                $context = \context_system::instance();
-            }
-
-            // Trigger event for reading a message.
-            $event = \core\event\message_viewed::create(array(
-                'objectid' => $mua->id,
-                'userid' => $userid, // Using the user who read the message as they are the ones performing the action.
-                'context' => $context,
-                'relateduserid' => $message->useridfrom,
-                'other' => array(
-                    'messageid' => $message->id
-                )
-            ));
-            $event->trigger();
+        $mua = new \stdClass();
+        $mua->userid = $userid;
+        $mua->messageid = $message->id;
+        $mua->action = self::MESSAGE_ACTION_READ;
+        $mua->timecreated = $timeread;
+        $mua->id = $DB->insert_record('message_user_actions', $mua);
+
+        // Get the context for the user who received the message.
+        $context = \context_user::instance($userid, IGNORE_MISSING);
+        // If the user no longer exists the context value will be false, in this case use the system context.
+        if ($context === false) {
+            $context = \context_system::instance();
         }
+
+        // Trigger event for reading a message.
+        $event = \core\event\message_viewed::create(array(
+            'objectid' => $mua->id,
+            'userid' => $userid, // Using the user who read the message as they are the ones performing the action.
+            'context' => $context,
+            'relateduserid' => $message->useridfrom,
+            'other' => array(
+                'messageid' => $message->id
+            )
+        ));
+        $event->trigger();
     }
 
     /**
index 528ace7..26c4ccd 100644 (file)
@@ -1922,9 +1922,16 @@ class core_message_external extends external_api {
                     ON m.conversationid = mc.id
             INNER JOIN {message_conversation_members} mcm
                     ON mcm.conversationid = mc.id
-                 WHERE mcm.userid != m.useridfrom
+             LEFT JOIN {message_user_actions} mua
+                    ON (mua.messageid = m.id AND mua.userid = ? AND mua.action = ?)
+                 WHERE mua.id is NULL
+                   AND mcm.userid != m.useridfrom
                    AND m.id = ?";
-        $message = $DB->get_record_sql($sql, [$params['messageid']], MUST_EXIST);
+        $messageparams = [];
+        $messageparams[] = $USER->id;
+        $messageparams[] = \core_message\api::MESSAGE_ACTION_READ;
+        $messageparams[] = $params['messageid'];
+        $message = $DB->get_record_sql($sql, $messageparams, MUST_EXIST);
 
         if ($message->useridto != $USER->id) {
             throw new invalid_parameter_exception('Invalid messageid, you don\'t have permissions to mark this message as read');