From 510ede569890929cada39a4c7992d5491283a026 Mon Sep 17 00:00:00 2001
From: Jake Dallimore <jake@moodle.com>
Date: Wed, 15 May 2019 18:15:56 +0800
Subject: [PATCH] MDL-65566 core_message: fix bug allowing duplicate unique
 conversations

Individual and self conversations may exist once only, so we need to
enforce this in the creation method.
---
 message/classes/api.php            |  6 ++++++
 message/tests/api_test.php         | 27 +++++++++++++++++++++++++++
 message/tests/externallib_test.php | 14 +++++++++++++-
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/message/classes/api.php b/message/classes/api.php
index 42bfb62217c..1af93dc3168 100644
--- a/message/classes/api.php
+++ b/message/classes/api.php
@@ -2528,6 +2528,12 @@ class api {
         $conversation->convhash = null;
         if ($type == self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL || $type == self::MESSAGE_CONVERSATION_TYPE_SELF) {
             $conversation->convhash = helper::get_conversation_hash($userids);
+
+            // Don't blindly create a conversation between 2 users if there is already one present - return that.
+            // This stops us making duplicate self and individual conversations, which is invalid.
+            if ($record = $DB->get_record('message_conversations', ['convhash' => $conversation->convhash])) {
+                return $record;
+            }
         }
         $conversation->component = $component;
         $conversation->itemtype = $itemtype;
diff --git a/message/tests/api_test.php b/message/tests/api_test.php
index 8d926458507..235ad5fa14c 100644
--- a/message/tests/api_test.php
+++ b/message/tests/api_test.php
@@ -2266,6 +2266,22 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         );
     }
 
+    /**
+     * Test that creation can't create the same conversation twice for 1:1 conversations.
+     */
+    public function test_create_conversation_duplicate_conversations() {
+        global $DB;
+        $user1 = $this::getDataGenerator()->create_user();
+
+        \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_SELF, [$user1->id]);
+        \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_SELF, [$user1->id]);
+
+        $convhash = \core_message\helper::get_conversation_hash([$user1->id]);
+        $countconversations = $DB->count_records('message_conversations', ['convhash' => $convhash]);
+        $this->assertEquals(1, $countconversations);
+        $this->assertNotEmpty($conversation = \core_message\api::get_self_conversation($user1->id));
+    }
+
     /**
      * Test get_conversations with a mixture of messages.
      *
@@ -6717,6 +6733,17 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
                 $messageids[] = testhelper::send_fake_message_to_conversation($userfrom, $conversation->id);
             }
 
+            // Remove the self conversations created by the generator,
+            // so we can choose to set that ourself and honour the original intention of the test.
+            $userids = array_map(function($userindex) use ($users) {
+                return $users[$userindex]->id;
+            }, $config['users']);
+            foreach ($userids as $userid) {
+                if ($conversation->type == \core_message\api::MESSAGE_CONVERSATION_TYPE_SELF) {
+                    \core_message\api::unset_favourite_conversation($conversation->id, $userid);
+                }
+            }
+
             foreach ($config['favourites'] as $userfromindex) {
                 $userfrom = $users[$userfromindex];
                 $usercontext = \context_user::instance($userfrom->id);
diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php
index e13591efe12..207eea78be7 100644
--- a/message/tests/externallib_test.php
+++ b/message/tests/externallib_test.php
@@ -6523,13 +6523,14 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
 
         $user1 = self::getDataGenerator()->create_user();
         $user2 = self::getDataGenerator()->create_user();
+        $user3 = self::getDataGenerator()->create_user();
 
         // Some random conversation.
         $otherconversation = \core_message\api::create_conversation(
             \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
             [
                 $user1->id,
-                $user2->id,
+                $user3->id,
             ]
         );
 
@@ -7039,6 +7040,17 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
                 $messageids[] = testhelper::send_fake_message_to_conversation($userfrom, $conversation->id);
             }
 
+            // Remove the self conversations created by the generator,
+            // so we can choose to set that ourself and honour the original intention of the test.
+            $userids = array_map(function($userindex) use ($users) {
+                return $users[$userindex]->id;
+            }, $config['users']);
+            foreach ($userids as $userid) {
+                if ($conversation->type == \core_message\api::MESSAGE_CONVERSATION_TYPE_SELF) {
+                    \core_message\api::unset_favourite_conversation($conversation->id, $userid);
+                }
+            }
+
             foreach ($config['favourites'] as $userfromindex) {
                 $userfrom = $users[$userfromindex];
                 $usercontext = \context_user::instance($userfrom->id);
-- 
2.17.1