MDL-36941 core_message: deprecated 'api::mark_all_read_for_user'

lib/deprecatedlib.php

@@ -4973,9 +4973,9 @@ function message_get_contact_block_link($incontactlist, $isblocked, $contact, $s
  */
 function message_mark_messages_read($touserid, $fromuserid) {
     debugging('message_mark_messages_read() is deprecated and is no longer used, please use
-        \core_message\api::mark_all_read_for_user() instead.', DEBUG_DEVELOPER);
+        \core_message\api::mark_all_messages_as_read() instead.', DEBUG_DEVELOPER);
 
-    \core_message\api::mark_all_read_for_user($touserid, $fromuserid);
+    \core_message\api::mark_all_messages_as_read($touserid, $fromuserid);
 }
 
 /**

message/classes/api.php

@@ -707,63 +707,108 @@ class api {
     }
 
     /**
-     * Marks ALL messages being sent from $fromuserid to $touserid as read.
+     * Marks all messages being sent to a user in a particular conversation.
      *
-     * Can be filtered by type.
+     * If $conversationdid is null then it marks all messages as read sent to $userid.
      *
-     * @param int $touserid the id of the message recipient
+     * @param int $userid
-     * @param int $fromuserid the id of the message sender
+     * @param int|null $conversationid The conversation the messages belong to mark as read, if null mark all
-     * @param string $type filter the messages by type, either MESSAGE_TYPE_NOTIFICATION, MESSAGE_TYPE_MESSAGE or '' for all.
-     * @return void
      */
-    public static function mark_all_read_for_user($touserid, $fromuserid = 0, $type = '') {
+    public static function mark_all_messages_as_read($userid, $conversationid = null) {
         global $DB;
 
-        $type = strtolower($type);
-
-        // Create the SQL we will be using.
         $messagesql = "SELECT m.*
                          FROM {messages} m
                    INNER JOIN {message_conversations} mc
                            ON mc.id = m.conversationid
                    INNER JOIN {message_conversation_members} mcm
                            ON mcm.conversationid = mc.id
-                        WHERE mcm.userid = ?";
+                        WHERE mcm.userid = ?
-        $messageparams = [$touserid];
+                          AND m.useridfrom != ?";
-        if (!empty($fromuserid)) {
+        $messageparams[] = $userid;
-            $messagesql .= " AND m.useridfrom = ?";
+        $messageparams[] = $userid;
-            $messageparams[] = $fromuserid;
+        if (!is_null($conversationid)) {
+            $messagesql .= " AND mc.id = ?";
+            $messageparams[] = $conversationid;
         }
 
+        $messages = $DB->get_recordset_sql($messagesql, $messageparams);
+        foreach ($messages as $message) {
+            self::mark_message_as_read($userid, $message);
+        }
+        $messages->close();
+    }
+
+    /**
+     * Marks all notifications being sent from one user to another user as read.
+     *
+     * If the from user is null then it marks all notifications as read sent to the to user.
+     *
+     * @param int $touserid the id of the message recipient
+     * @param int|null $fromuserid the id of the message sender, null if all messages
+     * @return void
+     */
+    public static function mark_all_notifications_as_read($touserid, $fromuserid = null) {
+        global $DB;
+
         $notificationsql = "SELECT n.*
                               FROM {notifications} n
-                             WHERE useridto = ?";
+                             WHERE useridto = ?
+                               AND timeread is NULL";
         $notificationsparams = [$touserid];
         if (!empty($fromuserid)) {
             $notificationsql .= " AND useridfrom = ?";
             $notificationsparams[] = $fromuserid;
         }
 
-        $messages = [];
-        $notifications = [];
-        if (!empty($type)) {
-            if (strtolower($type) == MESSAGE_TYPE_NOTIFICATION) {
         $notifications = $DB->get_recordset_sql($notificationsql, $notificationsparams);
-            } else if (strtolower($type) == MESSAGE_TYPE_MESSAGE) {
-                $messages = $DB->get_recordset_sql($messagesql, $messageparams);
-            }
-        } else { // We want both.
-            $messages = $DB->get_recordset_sql($messagesql, $messageparams);
-            $notifications = $DB->get_recordset_sql($notificationsql, $notificationsparams);
-        }
-
-        foreach ($messages as $message) {
-            self::mark_message_as_read($touserid, $message);
-        }
-
         foreach ($notifications as $notification) {
             self::mark_notification_as_read($notification);
         }
+        $notifications->close();
+    }
+
+    /**
+     * Marks ALL messages being sent from $fromuserid to $touserid as read.
+     *
+     * Can be filtered by type.
+     *
+     * @deprecated since 3.5
+     * @param int $touserid the id of the message recipient
+     * @param int $fromuserid the id of the message sender
+     * @param string $type filter the messages by type, either MESSAGE_TYPE_NOTIFICATION, MESSAGE_TYPE_MESSAGE or '' for all.
+     * @return void
+     */
+    public static function mark_all_read_for_user($touserid, $fromuserid = 0, $type = '') {
+        debugging('\core_message\api::mark_all_read_for_user is deprecated. Please either use ' .
+            '\core_message\api::mark_all_notifications_read_for_user or \core_message\api::mark_all_messages_read_for_user',
+            DEBUG_DEVELOPER);
+
+        $type = strtolower($type);
+
+        $conversationid = null;
+        $ignoremessages = false;
+        if (!empty($fromuserid)) {
+            $conversationid = \core_message\api::get_conversation_between_users([$touserid, $fromuserid]);
+            if (!$conversationid) { // If there is no conversation between the users then there are no messages to mark.
+                $ignoremessages = true;
+            }
+        }
+
+        if (!empty($type)) {
+            if ($type == MESSAGE_TYPE_NOTIFICATION) {
+                \core_message\api::mark_all_notifications_as_read($touserid, $fromuserid);
+            } else if ($type == MESSAGE_TYPE_MESSAGE) {
+                if (!$ignoremessages) {
+                    \core_message\api::mark_all_messages_as_read($touserid, $conversationid);
+                }
+            }
+        } else { // We want both.
+            \core_message\api::mark_all_notifications_as_read($touserid, $fromuserid);
+            if (!$ignoremessages) {
+                \core_message\api::mark_all_messages_as_read($touserid, $conversationid);
+            }
+        }
     }
 
     /**

message/externallib.php

@@ -1676,7 +1676,7 @@ class core_message_external extends external_api {
             throw new moodle_exception('accessdenied', 'admin');
         }
 
-        \core_message\api::mark_all_read_for_user($useridto, $useridfrom, MESSAGE_TYPE_NOTIFICATION);
+        \core_message\api::mark_all_notifications_as_read($useridto, $useridfrom);
 
         return true;
     }
@@ -2108,7 +2108,13 @@ class core_message_external extends external_api {
             throw new moodle_exception('accessdenied', 'admin');
         }
 
-        \core_message\api::mark_all_read_for_user($useridto, $useridfrom, MESSAGE_TYPE_MESSAGE);
+        if ($useridfrom) {
+            if ($conversationid = \core_message\api::get_conversation_between_users([$useridto, $useridfrom])) {
+                \core_message\api::mark_all_messages_as_read($useridto, $conversationid);
+            }
+        } else {
+            \core_message\api::mark_all_messages_as_read($useridto);
+        }
 
         return true;
     }

message/index.php

@@ -127,7 +127,10 @@ if (!$user2realuser) {
 if (!empty($user2->id)) {
     if ($currentuser && isset($conversations[$user2->id])) {
         // Mark the conversation we are loading as read.
-        \core_message\api::mark_all_read_for_user($user1->id, $user2->id);
+        if ($conversationid = \core_message\api::get_conversation_between_users([$user1->id, $user2->id])) {
+            \core_message\api::mark_all_messages_as_read($user1->id, $conversationid);
+        }
+
         // Ensure the UI knows it's read as well.
         $conversations[$user2->id]->isread = 1;
     }

message/tests/api_test.php

@@ -51,6 +51,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->send_fake_message($sender, $recipient);
 
         \core_message\api::mark_all_read_for_user($recipient->id);
+        $this->assertDebuggingCalled();
         $this->assertEquals(message_count_unread_messages($recipient), 0);
     }
 
@@ -73,6 +74,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->send_fake_message($sender2, $recipient);
 
         \core_message\api::mark_all_read_for_user($recipient->id, $sender1->id);
+        $this->assertDebuggingCalled();
         $this->assertEquals(message_count_unread_messages($recipient), 3);
     }
 
@@ -88,9 +90,11 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->send_fake_message($sender, $recipient);
 
         \core_message\api::mark_all_read_for_user($recipient->id, 0, MESSAGE_TYPE_NOTIFICATION);
+        $this->assertDebuggingCalled();
         $this->assertEquals(message_count_unread_messages($recipient), 3);
 
         \core_message\api::mark_all_read_for_user($recipient->id, 0, MESSAGE_TYPE_MESSAGE);
+        $this->assertDebuggingCalled();
         $this->assertEquals(message_count_unread_messages($recipient), 0);
     }