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.
This commit is contained in:
Jake Dallimore 2019-05-15 18:15:56 +08:00
parent 0e0e1b4552
commit 510ede5698
3 changed files with 46 additions and 1 deletions

View File

@ -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;

View File

@ -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);

View File

@ -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);