diff --git a/lang/en/role.php b/lang/en/role.php index 5f26748fa96..d0d200347d8 100644 --- a/lang/en/role.php +++ b/lang/en/role.php @@ -155,6 +155,7 @@ $string['context'] = 'Context'; $string['course:activityvisibility'] = 'Hide/show activities'; $string['course:bulkmessaging'] = 'Send a message to many people'; $string['course:create'] = 'Create courses'; +$string['course:creategroupconversations'] = 'Create group conversations'; $string['course:delete'] = 'Delete courses'; $string['course:viewsuspendedusers'] = 'View suspended users'; $string['course:changecategory'] = 'Change course category'; diff --git a/lib/db/access.php b/lib/db/access.php index 406c46d1d9a..7ad389c0293 100644 --- a/lib/db/access.php +++ b/lib/db/access.php @@ -788,6 +788,16 @@ $capabilities = array( ) ), + 'moodle/course:creategroupconversations' => array( + 'riskbitmask' => RISK_XSS, + 'captype' => 'write', + 'contextlevel' => CONTEXT_COURSE, + 'archetypes' => array( + 'editingteacher' => CAP_ALLOW, + 'manager' => CAP_ALLOW + ) + ), + 'moodle/course:request' => array( 'captype' => 'write', 'contextlevel' => CONTEXT_SYSTEM, diff --git a/lib/db/install.xml b/lib/db/install.xml index 60aa82ef74b..360bf469f82 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -616,14 +616,17 @@ - + + + - + +
diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 6ec46a8b503..d60b286e4f9 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2565,5 +2565,45 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2018101800.00); } + if ($oldversion < 2018102200.00) { + // Add field 'type' to 'message_conversations'. + $table = new xmldb_table('message_conversations'); + $field = new xmldb_field('type', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, 1, 'id'); + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Add field 'name' to 'message_conversations'. + $field = new xmldb_field('name', XMLDB_TYPE_CHAR, '255', null, null, null, null, 'type'); + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Conditionally launch add index 'type'. + $index = new xmldb_index('type', XMLDB_INDEX_NOTUNIQUE, ['type']); + if (!$dbman->index_exists($table, $index)) { + $dbman->add_index($table, $index); + } + + // Define table 'message_conversations' to be updated. + $table = new xmldb_table('message_conversations'); + + // Remove the unique 'convhash' index, change to null and add a new non unique index. + $index = new xmldb_index('convhash', XMLDB_INDEX_UNIQUE, ['convhash']); + if ($dbman->index_exists($table, $index)) { + $dbman->drop_index($table, $index); + } + + $field = new xmldb_field('convhash', XMLDB_TYPE_CHAR, '40', null, null, null, null, 'name'); + $dbman->change_field_notnull($table, $field); + + $index = new xmldb_index('convhash', XMLDB_INDEX_NOTUNIQUE, ['convhash']); + if (!$dbman->index_exists($table, $index)) { + $dbman->add_index($table, $index); + } + + upgrade_main_savepoint(true, 2018102200.00); + } + return true; } diff --git a/lib/messagelib.php b/lib/messagelib.php index 16ccefef9d2..b808981d8ba 100644 --- a/lib/messagelib.php +++ b/lib/messagelib.php @@ -163,8 +163,14 @@ function message_send(\core\message\message $eventdata) { if (!$conversationid = \core_message\api::get_conversation_between_users([$eventdata->userfrom->id, $eventdata->userto->id])) { - $conversationid = \core_message\api::create_conversation_between_users([$eventdata->userfrom->id, - $eventdata->userto->id]); + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $eventdata->userfrom->id, + $eventdata->userto->id + ] + ); + $conversationid = $conversation->id; } $tabledata = new stdClass(); diff --git a/message/classes/api.php b/message/classes/api.php index b2bd57aae3a..3e4b25f8853 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -61,6 +61,16 @@ class api { */ const MESSAGE_PRIVACY_SITE = 2; + /** + * An individual conversation. + */ + const MESSAGE_CONVERSATION_TYPE_INDIVIDUAL = 1; + + /** + * A group conversation. + */ + const MESSAGE_CONVERSATION_TYPE_GROUP = 2; + /** * Handles searching for messages in the message area. * @@ -1336,7 +1346,11 @@ class api { $hash = helper::get_conversation_hash($userids); - if ($conversation = $DB->get_record('message_conversations', ['convhash' => $hash])) { + $params = [ + 'type' => self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + 'convhash' => $hash + ]; + if ($conversation = $DB->get_record('message_conversations', $params)) { return $conversation->id; } @@ -1346,27 +1360,82 @@ class api { /** * Creates a conversation between two users. * + * @deprecated since 3.6 * @param array $userids * @return int The id of the conversation */ public static function create_conversation_between_users(array $userids) { + debugging('\core_message\api::create_conversation_between_users is deprecated, please use ' . + '\core_message\api::create_conversation instead.', DEBUG_DEVELOPER); + + // This method was always used for individual conversations. + $conversation = self::create_conversation(self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $userids); + + return $conversation->id; + } + + /** + * Creates a conversation with selected users and messages. + * + * @param int $type The type of conversation + * @param int[] $userids The array of users to add to the conversation + * @param string $name The name of the conversation + * @return \stdClass + */ + public static function create_conversation(int $type, array $userids, string $name = null) { global $DB; + // Sanity check. + if ($type == self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL) { + if (count($userids) > 2) { + throw new \moodle_exception('An individual conversation can not have more than two users.'); + } + } + $conversation = new \stdClass(); - $conversation->convhash = helper::get_conversation_hash($userids); + $conversation->type = $type; + $conversation->name = $name; + $conversation->convhash = null; + if ($type == self::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL) { + $conversation->convhash = helper::get_conversation_hash($userids); + } $conversation->timecreated = time(); $conversation->id = $DB->insert_record('message_conversations', $conversation); - // Add members to this conversation. + // Add users to this conversation. + $arrmembers = []; foreach ($userids as $userid) { $member = new \stdClass(); $member->conversationid = $conversation->id; $member->userid = $userid; $member->timecreated = time(); - $DB->insert_record('message_conversation_members', $member); + $member->id = $DB->insert_record('message_conversation_members', $member); + + $arrmembers[] = $member; } - return $conversation->id; + $conversation->members = $arrmembers; + + return $conversation; + } + + /** + * Checks if a user can create a group conversation. + * + * @param int $userid The id of the user attempting to create the conversation + * @param \context $context The context they are creating the conversation from, most likely course context + * @return bool + */ + public static function can_create_group_conversation(int $userid, \context $context) : bool { + global $CFG; + + // If we can't message at all, then we can't create a conversation. + if (empty($CFG->messaging)) { + return false; + } + + // We need to check they have the capability to create the conversation. + return has_capability('moodle/course:creategroupconversations', $context, $userid); } /** diff --git a/message/classes/task/migrate_message_data.php b/message/classes/task/migrate_message_data.php index 34d481f2386..53e21822bf6 100644 --- a/message/classes/task/migrate_message_data.php +++ b/message/classes/task/migrate_message_data.php @@ -123,7 +123,14 @@ class migrate_message_data extends \core\task\adhoc_task { global $DB; if (!$conversationid = \core_message\api::get_conversation_between_users([$userid, $otheruserid])) { - $conversationid = \core_message\api::create_conversation_between_users([$userid, $otheruserid]); + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $userid, + $otheruserid + ] + ); + $conversationid = $conversation->id; } // First, get the rows from the 'message' table. diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 545f277ce55..fbd575ee54c 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -2200,6 +2200,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $user2 = self::getDataGenerator()->create_user(); $conversationid = \core_message\api::create_conversation_between_users([$user1->id, $user2->id]); + $this->assertDebuggingCalled(); $this->assertEquals($conversationid, \core_message\api::get_conversation_between_users([$user1->id, $user2->id])); @@ -2471,7 +2472,14 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); - $conversationid = \core_message\api::create_conversation_between_users([$user1->id, $user2->id]); + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $user1->id, + $user2->id + ] + ); + $conversationid = $conversation->id; $this->assertTrue(\core_message\api::is_user_in_conversation($user1->id, $conversationid)); } @@ -2484,11 +2492,125 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $user2 = self::getDataGenerator()->create_user(); $user3 = self::getDataGenerator()->create_user(); - $conversationid = \core_message\api::create_conversation_between_users([$user1->id, $user2->id]); + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $user1->id, + $user2->id + ] + ); + $conversationid = $conversation->id; $this->assertFalse(\core_message\api::is_user_in_conversation($user3->id, $conversationid)); } + /** + * Test can create a group conversation. + */ + public function test_can_create_group_conversation() { + global $CFG; + + $student = self::getDataGenerator()->create_user(); + $teacher = self::getDataGenerator()->create_user(); + $course = self::getDataGenerator()->create_course(); + + $coursecontext = context_course::instance($course->id); + + $this->getDataGenerator()->enrol_user($student->id, $course->id); + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, 'editingteacher'); + + // Disable messaging. + $CFG->messaging = 0; + $this->assertFalse(\core_message\api::can_create_group_conversation($student->id, $coursecontext)); + + // Re-enable messaging. + $CFG->messaging = 1; + + // Student shouldn't be able to. + $this->assertFalse(\core_message\api::can_create_group_conversation($student->id, $coursecontext)); + + // Teacher should. + $this->assertTrue(\core_message\api::can_create_group_conversation($teacher->id, $coursecontext)); + } + + /** + * Test creating an individual conversation. + */ + public function test_create_conversation_individual() { + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $user1->id, + $user2->id + ], + 'A conversation name' + ); + + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $conversation->type); + $this->assertEquals('A conversation name', $conversation->name); + $this->assertEquals(\core_message\helper::get_conversation_hash([$user1->id, $user2->id]), $conversation->convhash); + + $this->assertCount(2, $conversation->members); + + $member1 = array_shift($conversation->members); + $member2 = array_shift($conversation->members); + + $this->assertEquals($user1->id, $member1->userid); + $this->assertEquals($conversation->id, $member1->conversationid); + + $this->assertEquals($user2->id, $member2->userid); + $this->assertEquals($conversation->id, $member2->conversationid); + } + + /** + * Test creating a group conversation. + */ + public function test_create_conversation_group() { + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + $user3 = self::getDataGenerator()->create_user(); + + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, + [ + $user1->id, + $user2->id, + $user3->id + ], + 'A conversation name' + ); + + $this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $conversation->type); + $this->assertEquals('A conversation name', $conversation->name); + $this->assertNull($conversation->convhash); + + $this->assertCount(3, $conversation->members); + + $member1 = array_shift($conversation->members); + $member2 = array_shift($conversation->members); + $member3 = array_shift($conversation->members); + + $this->assertEquals($user1->id, $member1->userid); + $this->assertEquals($conversation->id, $member1->conversationid); + + $this->assertEquals($user2->id, $member2->userid); + $this->assertEquals($conversation->id, $member2->conversationid); + + $this->assertEquals($user3->id, $member3->userid); + $this->assertEquals($conversation->id, $member3->conversationid); + } + + /** + * Test creating an individual conversation with too many members. + */ + public function test_create_conversation_individual_too_many_members() { + $this->expectException('moodle_exception'); + \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, [1, 2, 3]); + } + /** * Comparison function for sorting contacts. * diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index aa7f61f8d52..2a929d7bcdc 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -74,8 +74,14 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { } if (!$conversationid = \core_message\api::get_conversation_between_users([$userfrom->id, $userto->id])) { - $conversationid = \core_message\api::create_conversation_between_users([$userfrom->id, - $userto->id]); + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $userfrom->id, + $userto->id + ] + ); + $conversationid = $conversation->id; } // Ok, send the message. diff --git a/message/tests/messagelib_test.php b/message/tests/messagelib_test.php index 0aaac073490..dbc7a0befbd 100644 --- a/message/tests/messagelib_test.php +++ b/message/tests/messagelib_test.php @@ -86,8 +86,14 @@ class core_message_messagelib_testcase extends advanced_testcase { } if (!$conversationid = \core_message\api::get_conversation_between_users([$userfrom->id, $userto->id])) { - $conversationid = \core_message\api::create_conversation_between_users([$userfrom->id, - $userto->id]); + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $userfrom->id, + $userto->id + ] + ); + $conversationid = $conversation->id; } // Ok, send the message. diff --git a/message/tests/privacy_provider_test.php b/message/tests/privacy_provider_test.php index 27e2232d133..2e15a7bb57a 100644 --- a/message/tests/privacy_provider_test.php +++ b/message/tests/privacy_provider_test.php @@ -663,8 +663,14 @@ class core_message_privacy_provider_testcase extends \core_privacy\tests\provide } if (!$conversationid = \core_message\api::get_conversation_between_users([$useridfrom, $useridto])) { - $conversationid = \core_message\api::create_conversation_between_users([$useridfrom, - $useridto]); + $conversation = \core_message\api::create_conversation( + \core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, + [ + $useridfrom, + $useridto + ] + ); + $conversationid = $conversation->id; } // Ok, send the message. diff --git a/message/upgrade.txt b/message/upgrade.txt index 6060c8b110e..5aae3c25bf0 100644 --- a/message/upgrade.txt +++ b/message/upgrade.txt @@ -30,6 +30,7 @@ information provided here is intended especially for developers. - \core_message\api::is_user_blocked() - \core_message\api::delete_conversation() - \core_message\api::is_user_non_contact_blocked() + - \core_message\api::create_conversation_between_users() * The method \core_message\api::can_delete_conversation() now expects a 'conversationid' to be passed as the second parameter. * The following web services have been deprecated. Please do not call these any more. diff --git a/version.php b/version.php index 82f0af37444..af76cef02d9 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2018101900.01; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2018102200.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.