diff --git a/group/group_form.php b/group/group_form.php index 79e8c7a9ca2..b92dcec58d9 100644 --- a/group/group_form.php +++ b/group/group_form.php @@ -96,7 +96,7 @@ class group_form extends moodleform { * Extend the form definition after the data has been parsed. */ public function definition_after_data() { - global $COURSE, $DB; + global $COURSE, $DB, $USER; $mform = $this->_form; $groupid = $mform->getElementValue('id'); @@ -104,11 +104,8 @@ class group_form extends moodleform { if ($group = $DB->get_record('groups', array('id' => $groupid))) { // If can create group conversation then get if a conversation area exists and it is enabled. - if ($mform->elementExists('enablemessaging')) { - if (\core_message\helper::get_does_conversation_area_enabled('core_group', - 'groups', - $groupid, - $coursecontext->id)) { + if (\core_message\api::can_create_group_conversation($USER->id, $coursecontext)) { + if (\core_message\api::is_conversation_area_enabled('core_group', 'groups', $groupid, $coursecontext->id)) { $mform->getElement('enablemessaging')->setSelected(1); } } diff --git a/group/lib.php b/group/lib.php index 5f36f9d1e46..e06ea2a8f23 100644 --- a/group/lib.php +++ b/group/lib.php @@ -278,12 +278,7 @@ function groups_create_group($data, $editform = false, $editoroptions = false) { // Group conversation messaging. if (\core_message\api::can_create_group_conversation($USER->id, $context)) { if (!empty($data->enablemessaging)) { - \core_message\api::create_conversation_area('core_group', - 'groups', - $group->id, - $context->id, - 1, - $group->name); + \core_message\api::create_conversation_area('core_group', 'groups', $group->id, $context->id, $group->name, 1); } } @@ -427,10 +422,7 @@ function groups_update_group($data, $editform = false, $editoroptions = false) { // Group conversation messaging. if (\core_message\api::can_create_group_conversation($USER->id, $context)) { - if ($conversationarea = \core_message\api::get_conversation_area('core_group', - 'groups', - $group->id, - $context->id)) { + if ($conversationarea = \core_message\api::get_conversation_area('core_group', 'groups', $group->id, $context->id)) { if ($data->enablemessaging && $data->enablemessaging != $conversationarea->enabled) { \core_message\api::enable_conversation_area($conversationarea->id); } @@ -440,12 +432,7 @@ function groups_update_group($data, $editform = false, $editoroptions = false) { \core_message\api::update_conversation_name($conversationarea->conversationid, $group->name); } else { if (!empty($data->enablemessaging)) { - \core_message\api::create_conversation_area('core_group', - 'groups', - $group->id, - $context->id, - 1, - $group->name); + \core_message\api::create_conversation_area('core_group', 'groups', $group->id, $context->id, $group->name, 1); } } } diff --git a/group/tests/lib_test.php b/group/tests/lib_test.php index 45d3d577edd..b141b68dee6 100644 --- a/group/tests/lib_test.php +++ b/group/tests/lib_test.php @@ -532,53 +532,39 @@ class core_group_lib_testcase extends advanced_testcase { /** * Test groups_create_group enabling a group of conversation. */ - public function groups_create_group_with_conversation_area() { + public function test_groups_create_group_with_conversation_area() { global $DB; $this->resetAfterTest(); $this->setAdminUser(); $course1 = $this->getDataGenerator()->create_course(); $coursecontext1 = context_course::instance($course1->id); - // Check not exists and conversation area created. - $this->assertEquals( - 0, - $DB->count_records_sql("SELECT COUNT(ca.id) - FROM {message_conversation_area} ca - WHERE ca.contextid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups'", [$coursecontext1->id]) - ); + // Create two groups and only one group with enablemessaging = 1. $group1a = $this->getDataGenerator()->create_group(array('courseid' => $course1->id, 'enablemessaging' => 1)); $group1b = $this->getDataGenerator()->create_group(array('courseid' => $course1->id, 'enablemessaging' => 0)); - // Check exist only one row created in conversation area. - $this->assertEquals( - 1, - $DB->count_records_sql("SELECT COUNT(ca.id) - FROM {message_conversation_area} ca - WHERE ca.contextid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups' - AND ca.enabled = 1", [$coursecontext1->id]) + + $conversationareas = $DB->get_records('message_conversation_area', + [ + 'contextid' => $coursecontext1->id, + 'component' => 'core_group', + 'itemtype' => 'groups', + 'enabled' => 1 + ] ); - $conversation = $DB->get_record_sql("SELECT c.* - FROM {message_conversations} c - JOIN {message_conversation_area} ca ON c.id = ca.conversationid - WHERE ca.contextid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups'", [$coursecontext1->id]); - // Check group name and course fullname was stored in conversation. - $this->assertEquals($group1a->name, $conversation->name); + $this->assertCount(1, $conversationareas); + + $conversationarea = reset($conversationareas); // Check groupid was stored in itemid on conversation area. - $this->assertEquals( - $group1a->id, - $DB->get_field_sql("SELECT ca.itemid - FROM {message_conversation_area} ca - WHERE ca.contextid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups' - AND ca.conversationid = ?", [$coursecontext1->id, $conversation->id]) - ); + $this->assertEquals($group1a->id, $conversationarea->itemid); + + $conversations = $DB->get_records('message_conversations', ['id' => $conversationarea->conversationid]); + $this->assertCount(1, $conversations); + + $conversation = reset($conversations); + + // Check group name was stored in conversation. + $this->assertEquals($group1a->name, $conversation->name); } /** @@ -591,69 +577,51 @@ class core_group_lib_testcase extends advanced_testcase { $this->setAdminUser(); $course1 = $this->getDataGenerator()->create_course(); $coursecontext1 = context_course::instance($course1->id); - // Check not exists and conversation area created. - $this->assertEquals( - 0, - $DB->count_records_sql("SELECT COUNT(ca.id) - FROM {message_conversation_area} ca - WHERE ca.contextid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups'", [$coursecontext1->id]) - ); + // Create two groups and only one group with enablemessaging = 1. $group1a = $this->getDataGenerator()->create_group(array('courseid' => $course1->id, 'enablemessaging' => 1)); $group1b = $this->getDataGenerator()->create_group(array('courseid' => $course1->id, 'enablemessaging' => 0)); - // Check exist only one row created in conversation area. - $this->assertEquals( - 1, - $DB->count_records_sql("SELECT COUNT(ca.id) - FROM {message_conversation_area} ca - WHERE ca.contextid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups' - AND ca.enabled = 1", [$coursecontext1->id]) + + $conversationareas = $DB->get_records('message_conversation_area', + [ + 'contextid' => $coursecontext1->id, + 'component' => 'core_group', + 'itemtype' => 'groups', + 'enabled' => 1 + ] ); + $this->assertCount(1, $conversationareas); + // Check that the conversation area is created when group messaging is enabled in the course group. $group1b->enablemessaging = 1; groups_update_group($group1b); - $this->assertEquals( - 2, - $DB->count_records_sql("SELECT COUNT(ca.id) - FROM {message_conversation_area} ca - WHERE ca.contextid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups' - AND ca.enabled = 1", [$coursecontext1->id]) - ); - $conversation1b = $DB->get_record_sql("SELECT c.* - FROM {message_conversations} c - JOIN {message_conversation_area} ca ON c.id = ca.conversationid - WHERE ca.contextid = ? - AND ca.itemid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups'", [$coursecontext1->id, $group1b->id]); + + $conversationareas = $DB->get_records('message_conversation_area', + [ + 'contextid' => $coursecontext1->id, + 'component' => 'core_group', + 'itemtype' => 'groups', + 'enabled' => 1 + ], + 'id ASC'); + $this->assertCount(2, $conversationareas); + + $conversationarea1a = array_shift($conversationareas); + $conversationarea1b = array_shift($conversationareas); + + $conversation1b = $DB->get_record('message_conversations', ['id' => $conversationarea1b->conversationid]); + // Check for group1b that group name was stored in conversation. $this->assertEquals($group1b->name, $conversation1b->name); - // Check how to is disabled conversation area when group messaging is disabled in the course group. - $this->assertEquals(1, $DB->get_field("message_conversation_area", "enabled", - ['itemid' => $group1b->id, - 'contextid' => $coursecontext1->id])); + $group1b->enablemessaging = 0; groups_update_group($group1b); - $this->assertEquals(0, $DB->get_field("message_conversation_area", "enabled", - ['itemid' => $group1b->id, - 'contextid' => $coursecontext1->id])); + $this->assertEquals(0, $DB->get_field("message_conversation_area", "enabled", ['id' => $conversationarea1b->id])); + // Check that the name of the conversation is changed when the name of the course group is updated. $group1b->name = 'New group name'; - $this->assertNotEquals($group1b->name, $conversation1b->name); groups_update_group($group1b); - $conversation1b = $DB->get_record_sql("SELECT c.* - FROM {message_conversations} c - JOIN {message_conversation_area} ca ON c.id = ca.conversationid - WHERE ca.contextid = ? - AND ca.itemid = ? - AND ca.component = 'core_group' - AND ca.itemtype = 'groups'", [$coursecontext1->id, $group1b->id]); + $conversation1b = $DB->get_record('message_conversations', ['id' => $conversation1b->id]); $this->assertEquals($group1b->name, $conversation1b->name); } } diff --git a/lib/db/install.xml b/lib/db/install.xml index 4e6290d22e6..3148f1485b6 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -636,7 +636,7 @@ - + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 68b92a30f7c..4c66eabacd7 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2589,7 +2589,7 @@ function xmldb_main_upgrade($oldversion) { $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, array('convhash')); + $index = new xmldb_index('convhash', XMLDB_INDEX_UNIQUE, ['convhash']); if ($dbman->index_exists($table, $index)) { $dbman->drop_index($table, $index); } @@ -2597,7 +2597,7 @@ function xmldb_main_upgrade($oldversion) { $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, array('convhash')); + $index = new xmldb_index('convhash', XMLDB_INDEX_NOTUNIQUE, ['convhash']); if (!$dbman->index_exists($table, $index)) { $dbman->add_index($table, $index); } @@ -2605,18 +2605,18 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2018102200.00); } - if ($oldversion < 2018101900.03) { + if ($oldversion < 2018102300.02) { // Create new 'message_conversation_area' table. $table = new xmldb_table('message_conversation_area'); $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null, null); $table->add_field('conversationid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null, 'id'); - $table->add_field('component', XMLDB_TYPE_CHAR, '100', null, null, null, null, 'conversationid'); - $table->add_field('itemtype', XMLDB_TYPE_CHAR, '100', null, null, null, null, 'component'); + $table->add_field('component', XMLDB_TYPE_CHAR, '100', null, XMLDB_NOTNULL, null, null, 'conversationid'); + $table->add_field('itemtype', XMLDB_TYPE_CHAR, '100', null, XMLDB_NOTNULL, null, null, 'component'); $table->add_field('itemid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null, 'itemtype'); - $table->add_field('contextid', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'itemid'); + $table->add_field('contextid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null, 'itemid'); $table->add_field('enabled', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, 0, 'contextid'); - $table->add_field('timecreated', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'enable'); - $table->add_field('timemodified', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'timecreated'); + $table->add_field('timecreated', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null, 'enabled'); + $table->add_field('timemodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null, 'timecreated'); $table->add_key('primary', XMLDB_KEY_PRIMARY, ['id'], null, null); $table->add_key('conversationid', XMLDB_KEY_FOREIGN, ['conversationid'], 'message_conversations', ['id']); $table->add_key('contextid', XMLDB_KEY_FOREIGN, ['contextid'], 'context', ['id']); @@ -2628,7 +2628,7 @@ function xmldb_main_upgrade($oldversion) { $dbman->create_table($table); } - upgrade_main_savepoint(true, 2018101900.03); + upgrade_main_savepoint(true, 2018102300.02); } return true; diff --git a/message/classes/api.php b/message/classes/api.php index a32548ae634..b29bde37246 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -1983,24 +1983,21 @@ class api { * @param string $itemtype Defines the type of the component * @param int $itemid The id of the component * @param int $contextid The id of the context - * @param int $enabled Allow enabled or disabled the conversation area * @param string $name The main name of the area + * @param int $enabled Enable or disable the conversation area * @return \stdClass */ - public static function create_conversation_area(string $component, - string $itemtype, - int $itemid, - int $contextid, - int $enabled = 0, - string $name) { + public static function create_conversation_area(string $component, string $itemtype, int $itemid, int $contextid, string $name, + int $enabled = 0) { global $DB; // Create a conversation. $conversation = self::create_conversation(self::MESSAGE_CONVERSATION_TYPE_GROUP, array(), $name); + // Create a conversation area. $conversationarea = new \stdClass; $conversationarea->conversationid = $conversation->id; - $conversationarea->component = !empty($component) ? $component : ''; + $conversationarea->component = $component; $conversationarea->itemtype = $itemtype; $conversationarea->itemid = $itemid; $conversationarea->contextid = $contextid; @@ -2012,6 +2009,29 @@ class api { return $conversationarea; } + /** + * Checks whether or not a conversation area is enabled. + * + * @param string $component Defines the Moodle component which the area was added to. + * @param string $itemtype Defines the type of the component. + * @param int $itemid The id of the component. + * @param int $contextid The id of the context. + * @return bool Returns if a conversation area exists and is enabled, false otherwise + */ + public static function is_conversation_area_enabled(string $component, string $itemtype, int $itemid, int $contextid) : bool { + global $DB; + + return $DB->record_exists('message_conversation_area', + [ + 'itemid' => $itemid, + 'contextid' => $contextid, + 'component' => $component, + 'itemtype' => $itemtype, + 'enabled' => 1 + ] + ); + } + /** * Get conversation area. * @@ -2021,16 +2041,17 @@ class api { * @param int $contextid The id of the context. * @return object message_conversation_area. */ - public static function get_conversation_area(string $component, - string $itemtype, - int $itemid, - int $contextid) { + public static function get_conversation_area(string $component, string $itemtype, int $itemid, int $contextid) { global $DB; - return $DB->get_record('message_conversation_area', ['itemid' => $itemid, - 'contextid' => $contextid, - 'component' => $component, - 'itemtype' => $itemtype]); + return $DB->get_record('message_conversation_area', + [ + 'itemid' => $itemid, + 'contextid' => $contextid, + 'component' => $component, + 'itemtype' => $itemtype + ] + ); } /** @@ -2045,7 +2066,7 @@ class api { $conversationarea = new \stdClass(); $conversationarea->id = $conversationareaid; $conversationarea->enabled = 1; - $conversationarea->timeread = time(); + $conversationarea->timemodified = time(); $DB->update_record('message_conversation_area', $conversationarea); } @@ -2061,7 +2082,7 @@ class api { $conversationarea = new \stdClass(); $conversationarea->id = $conversationareaid; $conversationarea->enabled = 0; - $conversationarea->timeread = time(); + $conversationarea->timemodified = time(); $DB->update_record('message_conversation_area', $conversationarea); } @@ -2078,6 +2099,7 @@ class api { if ($conversation = $DB->get_record('message_conversations', array('id' => $conversationid))) { if ($name <> $conversation->name) { $conversation->name = $name; + $conversation->timemodified = time(); $DB->update_record('message_conversations', $conversation); } } diff --git a/message/classes/helper.php b/message/classes/helper.php index a2eab34d801..cf5a8b68b6d 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -354,26 +354,4 @@ class helper { return $messageexists || $messagereadexists; } - - /** - * Get if exists a conversation area and is enabled. - * - * @param string $component Defines the Moodle component which the area was added to. - * @param string $itemtype Defines the type of the component. - * @param int $itemid The id of the component. - * @param int $contextid The id of the context. - * @return bool Returns if a conversation area exists and is enabled, false otherwise - */ - public static function get_does_conversation_area_enabled(string $component, - string $itemtype, - int $itemid, - int $contextid) : bool { - global $DB; - - return $DB->record_exists('message_conversation_area', ['itemid' => $itemid, - 'contextid' => $contextid, - 'component' => $component, - 'itemtype' => $itemtype, - 'enabled' => 1]); - } } diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 1ce95f4b0b3..da8525b8422 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -3263,64 +3263,33 @@ class core_message_api_testcase extends core_message_messagelib_testcase { \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, [1, 2, 3]); } - /** - * Comparison function for sorting contacts. - * - * @param stdClass $a - * @param stdClass $b - * @return bool - */ - protected static function sort_contacts($a, $b) { - return $a->userid > $b->userid; - } - /** * Test create message conversation area. */ public function test_create_conversation_area() { - global $DB; - $this->resetAfterTest(); $contextid = 111; $itemid = 222; $name = 'Name of conversation'; - \core_message\api::create_conversation_area('core_group', - 'groups', - $itemid, - $contextid, - 1, - $name); - $conversationarea = $DB->get_record('message_conversation_area', ['itemid' => $itemid, - 'contextid' => $contextid, - 'component' => 'core_group', - 'itemtype' => 'groups']); - // Check if exist the new conversation area and if the conversation has been created with the name. + $conversationarea = \core_message\api::create_conversation_area('core_group', 'groups', $itemid, $contextid, $name, 0); + $this->assertEquals($itemid, $conversationarea->itemid); - $this->assertEquals( - $name, - $DB->get_field('message_conversations', 'name', ['id' => $conversationarea->conversationid]) - ); + $this->assertEquals($contextid, $conversationarea->contextid); + $this->assertEquals('core_group', $conversationarea->component); + $this->assertEquals('groups', $conversationarea->itemtype); + $this->assertEquals(0, $conversationarea->enabled); } /** * Test get_conversation_area. */ public function test_get_conversation_area() { - global $DB; - $contextid = 111; $itemid = 222; $name = 'Name of conversation'; - \core_message\api::create_conversation_area('core_group', - 'groups', - $itemid, - $contextid, - 1, - $name); - $conversationarea = \core_message\api::get_conversation_area('core_group', - 'groups', - $itemid, - $contextid); + \core_message\api::create_conversation_area('core_group', 'groups', $itemid, $contextid, $name, 1); + $conversationarea = \core_message\api::get_conversation_area('core_group', 'groups', $itemid, $contextid); + $this->assertEquals($itemid, $conversationarea->itemid); $this->assertEquals($contextid, $conversationarea->contextid); $this->assertEquals('core_group', $conversationarea->component); @@ -3337,23 +3306,10 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $contextid = 111; $itemid = 222; $name = 'Name of conversation'; - \core_message\api::create_conversation_area('core_group', - 'groups', - $itemid, - $contextid, - 0, - $name); - $conversationarea = $DB->get_record('message_conversation_area', ['itemid' => $itemid, - 'contextid' => $contextid, - 'component' => 'core_group', - 'itemtype' => 'groups']); - $this->assertEquals(0, $conversationarea->enabled); + $conversationarea = \core_message\api::create_conversation_area('core_group', 'groups', $itemid, $contextid, $name, 0); \core_message\api::enable_conversation_area($conversationarea->id); - $conversationarea = $DB->get_record('message_conversation_area', ['itemid' => $itemid, - 'contextid' => $contextid, - 'component' => 'core_group', - 'itemtype' => 'groups']); - $this->assertEquals(1, $conversationarea->enabled); + $conversationareaenabled = $DB->get_field('message_conversation_area', 'enabled', ['id' => $conversationarea->id]); + $this->assertEquals(1, $conversationareaenabled); } /** @@ -3365,23 +3321,11 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $contextid = 111; $itemid = 222; $name = 'Name of conversation'; - \core_message\api::create_conversation_area('core_group', - 'groups', - $itemid, - $contextid, - 1, - $name); - $conversationarea = $DB->get_record('message_conversation_area', ['itemid' => $itemid, - 'contextid' => $contextid, - 'component' => 'core_group', - 'itemtype' => 'groups']); + $conversationarea = \core_message\api::create_conversation_area('core_group', 'groups', $itemid, $contextid, $name, 1); $this->assertEquals(1, $conversationarea->enabled); \core_message\api::disable_conversation_area($conversationarea->id); - $conversationarea = $DB->get_record('message_conversation_area', ['itemid' => $itemid, - 'contextid' => $contextid, - 'component' => 'core_group', - 'itemtype' => 'groups']); - $this->assertEquals(0, $conversationarea->enabled); + $conversationareaenabled = $DB->get_field('message_conversation_area', 'enabled', ['id' => $conversationarea->id]); + $this->assertEquals(0, $conversationareaenabled); } /** * Test update_conversation_name. @@ -3392,20 +3336,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $contextid = 111; $itemid = 222; $name = 'Name of conversation'; - \core_message\api::create_conversation_area('core_group', - 'groups', - $itemid, - $contextid, - 1, - $name); - $conversationarea = $DB->get_record('message_conversation_area', ['itemid' => $itemid, - 'contextid' => $contextid, - 'component' => 'core_group', - 'itemtype' => 'groups']); - $this->assertEquals( - $name, - $DB->get_field('message_conversations', 'name', ['id' => $conversationarea->conversationid]) - ); + $conversationarea = \core_message\api::create_conversation_area('core_group', 'groups', $itemid, $contextid, $name, 1); $newname = 'New name of conversation'; \core_message\api::update_conversation_name($conversationarea->conversationid, $newname); @@ -3415,4 +3346,15 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $DB->get_field('message_conversations', 'name', ['id' => $conversationarea->conversationid]) ); } + + /** + * Comparison function for sorting contacts. + * + * @param stdClass $a + * @param stdClass $b + * @return bool + */ + protected static function sort_contacts($a, $b) { + return $a->userid > $b->userid; + } } diff --git a/version.php b/version.php index 36712286aaa..c978800be86 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2018102300.01; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2018102300.02; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.