From 08cb8a34f999830c9722abd928f4c72a4f8b91fd Mon Sep 17 00:00:00 2001
From: Mark Nelson <markn@moodle.com>
Date: Sun, 14 Oct 2018 19:48:13 +0800
Subject: [PATCH] MDL-63547 core_message: updated api::can_delete_message

---
 message/classes/api.php            | 45 +++++++++++++++++-------------
 message/tests/api_test.php         | 25 +++++++++++++++++
 message/tests/externallib_test.php |  2 +-
 3 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/message/classes/api.php b/message/classes/api.php
index d8b07950b4e..4320e25ea89 100644
--- a/message/classes/api.php
+++ b/message/classes/api.php
@@ -1197,30 +1197,20 @@ class api {
     public static function can_delete_message($userid, $messageid) {
         global $DB, $USER;
 
-        $sql = "SELECT m.id, m.useridfrom, mcm.userid as useridto
-                  FROM {messages} m
-            INNER JOIN {message_conversations} mc
-                    ON m.conversationid = mc.id
-            INNER JOIN {message_conversation_members} mcm
-                    ON mcm.conversationid = mc.id
-                 WHERE mcm.userid != m.useridfrom
-                   AND m.id = ?";
-        $message = $DB->get_record_sql($sql, [$messageid], MUST_EXIST);
+        $systemcontext = \context_system::instance();
 
-        if ($message->useridfrom == $userid) {
-            $userdeleting = 'useridfrom';
-        } else if ($message->useridto == $userid) {
-            $userdeleting = 'useridto';
-        } else {
+        $conversationid = $DB->get_field('messages', 'conversationid', ['id' => $messageid], MUST_EXIST);
+
+        if (has_capability('moodle/site:deleteanymessage', $systemcontext)) {
+            return true;
+        }
+
+        if (!self::is_user_in_conversation($userid, $conversationid)) {
             return false;
         }
 
-        $systemcontext = \context_system::instance();
-
-        // Let's check if the user is allowed to delete this message.
-        if (has_capability('moodle/site:deleteanymessage', $systemcontext) ||
-            ((has_capability('moodle/site:deleteownmessage', $systemcontext) &&
-                $USER->id == $message->$userdeleting))) {
+        if (has_capability('moodle/site:deleteownmessage', $systemcontext) &&
+                $USER->id == $userid) {
             return true;
         }
 
@@ -1601,4 +1591,19 @@ class api {
                     OR (mcr.userid = ? AND mcr.requesteduserid = ?)";
         return $DB->record_exists_sql($sql, [$userid, $requesteduserid, $requesteduserid, $userid]);
     }
+
+    /**
+     * Checks if a user is already in a conversation.
+     *
+     * @param int $userid The id of the user we want to check if they are in a group
+     * @param int $conversationid The id of the conversation
+     * @return bool Returns true if a contact request exists, false otherwise
+     */
+    public static function is_user_in_conversation(int $userid, int $conversationid) : bool {
+        global $DB;
+
+        return $DB->record_exists('message_conversation_members', ['conversationid' => $conversationid,
+            'userid' => $userid]);
+
+    }
 }
diff --git a/message/tests/api_test.php b/message/tests/api_test.php
index 0278d679352..f9f1cfc15c2 100644
--- a/message/tests/api_test.php
+++ b/message/tests/api_test.php
@@ -2226,6 +2226,31 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         $this->assertTrue(\core_message\api::does_contact_request_exist($user2->id, $user1->id));
     }
 
+    /**
+     * Test the user in conversation check.
+     */
+    public function test_is_user_in_conversation() {
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        $conversationid = \core_message\api::create_conversation_between_users([$user1->id, $user2->id]);
+
+        $this->assertTrue(\core_message\api::is_user_in_conversation($user1->id, $conversationid));
+    }
+
+    /**
+     * Test the user in conversation check when they are not.
+     */
+    public function test_is_user_in_conversation_when_not() {
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+        $user3 = self::getDataGenerator()->create_user();
+
+        $conversationid = \core_message\api::create_conversation_between_users([$user1->id, $user2->id]);
+
+        $this->assertFalse(\core_message\api::is_user_in_conversation($user3->id, $conversationid));
+    }
+
     /**
      * Comparison function for sorting contacts.
      *
diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php
index 784ad149ee2..6b503e5c7de 100644
--- a/message/tests/externallib_test.php
+++ b/message/tests/externallib_test.php
@@ -1648,7 +1648,7 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
             $result = core_message_external::delete_message(-1, $user1->id);
             $this->fail('Exception expected due invalid messageid.');
         } catch (dml_missing_record_exception $e) {
-            $this->assertEquals('invalidrecordunknown', $e->errorcode);
+            $this->assertEquals('invalidrecord', $e->errorcode);
         }
 
         // Invalid user.