From 1c059cb3fe39da46959e912dc671844dd204e83b Mon Sep 17 00:00:00 2001 From: Ilya Tregubov Date: Thu, 8 Feb 2024 10:51:27 +0700 Subject: [PATCH] MDL-80504 forum: Fix seperate group mode --- enrol/externallib.php | 29 +++-- enrol/locallib.php | 22 +++- enrol/tests/course_enrolment_manager_test.php | 69 ++++++++-- enrol/tests/externallib_test.php | 66 ++++++++++ enrol/upgrade.txt | 2 + mod/forum/amd/build/form-user-selector.min.js | 2 +- .../amd/build/form-user-selector.min.js.map | 2 +- mod/forum/amd/src/form-user-selector.js | 4 +- mod/forum/classes/form/export_form.php | 2 +- mod/forum/classes/local/vaults/discussion.php | 21 ++- mod/forum/export.php | 3 +- mod/forum/tests/behat/forum_export.feature | 122 ++++++++++++++++++ 12 files changed, 309 insertions(+), 35 deletions(-) diff --git a/enrol/externallib.php b/enrol/externallib.php index 4cb5e0c4b24..4ee08386251 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -629,6 +629,7 @@ class core_enrol_external extends external_api { 'searchanywhere' => new external_value(PARAM_BOOL, 'find a match anywhere, or only at the beginning'), 'page' => new external_value(PARAM_INT, 'Page number'), 'perpage' => new external_value(PARAM_INT, 'Number per page'), + 'contextid' => new external_value(PARAM_INT, 'Context ID', VALUE_DEFAULT, null), ] ); } @@ -641,11 +642,12 @@ class core_enrol_external extends external_api { * @param bool $searchanywhere Match anywhere in the string * @param int $page Page number * @param int $perpage Max per page + * @param ?int $contextid Context ID we are in - we might use search on activity level and its group mode can be different from course group mode. * @return array An array of users * @throws moodle_exception */ - public static function search_users(int $courseid, string $search, bool $searchanywhere, int $page, int $perpage): array { - global $PAGE, $DB, $CFG; + public static function search_users(int $courseid, string $search, bool $searchanywhere, int $page, int $perpage, ?int $contextid = null): array { + global $PAGE, $CFG; require_once($CFG->dirroot.'/enrol/locallib.php'); require_once($CFG->dirroot.'/user/lib.php'); @@ -657,10 +659,15 @@ class core_enrol_external extends external_api { 'search' => $search, 'searchanywhere' => $searchanywhere, 'page' => $page, - 'perpage' => $perpage - ] + 'perpage' => $perpage, + 'contextid' => $contextid, + ], ); - $context = context_course::instance($params['courseid']); + if (isset($contextid)) { + $context = context::instance_by_id($params['contextid']); + } else { + $context = context_course::instance($params['courseid']); + } try { self::validate_context($context); } catch (Exception $e) { @@ -674,10 +681,14 @@ class core_enrol_external extends external_api { $course = get_course($params['courseid']); $manager = new course_enrolment_manager($PAGE, $course); - $users = $manager->search_users($params['search'], - $params['searchanywhere'], - $params['page'], - $params['perpage']); + $users = $manager->search_users( + $params['search'], + $params['searchanywhere'], + $params['page'], + $params['perpage'], + false, + $params['contextid'] + ); $results = []; // Add also extra user fields. diff --git a/enrol/locallib.php b/enrol/locallib.php index dd2818c582d..5bcd05b30fe 100644 --- a/enrol/locallib.php +++ b/enrol/locallib.php @@ -563,26 +563,40 @@ class course_enrolment_manager { * @param int $page Starting at 0. * @param int $perpage Number of users returned per page. * @param bool $returnexactcount Return the exact total users using count_record or not. + * @param ?int $contextid Context ID we are in - we might use search on activity level and its group mode can be different from course group mode. * @return array with two or three elements: * int totalusers Number users matching the search. (This element only exist if $returnexactcount was set to true) * array users List of user objects returned by the query. * boolean moreusers True if there are still more users, otherwise is False. */ public function search_users(string $search = '', bool $searchanywhere = false, int $page = 0, int $perpage = 25, - bool $returnexactcount = false) { + bool $returnexactcount = false, ?int $contextid = null) { global $USER; [$ufields, $joins, $params, $wherecondition] = $this->get_basic_search_conditions($search, $searchanywhere); - $groupmode = groups_get_course_groupmode($this->course); - if ($groupmode == SEPARATEGROUPS && !has_capability('moodle/site:accessallgroups', $this->context)) { + if (isset($contextid)) { + // If contextid is set, we need to determine the group mode that should be used (module or course). + [$context, $course, $cm] = get_context_info_array($contextid); + // If cm instance is returned, then use the group mode from the module, otherwise get the course group mode. + $groupmode = $cm ? groups_get_activity_groupmode($cm, $course) : groups_get_course_groupmode($this->course); + } else { + // Otherwise, default to the group mode of the course. + $context = $this->context; + $groupmode = groups_get_course_groupmode($this->course); + } + + if ($groupmode == SEPARATEGROUPS && !has_capability('moodle/site:accessallgroups', $context)) { $groups = groups_get_all_groups($this->course->id, $USER->id, 0, 'g.id'); $groupids = array_column($groups, 'id'); + if (!$groupids) { + return ['totalusers' => 0, 'users' => [], 'moreusers' => false]; + } } else { $groupids = []; } - [$enrolledsql, $enrolledparams] = get_enrolled_sql($this->context, '', $groupids); + [$enrolledsql, $enrolledparams] = get_enrolled_sql($context, '', $groupids); $fields = 'SELECT ' . $ufields; $countfields = 'SELECT COUNT(u.id)'; diff --git a/enrol/tests/course_enrolment_manager_test.php b/enrol/tests/course_enrolment_manager_test.php index 91b90edae1a..01fb9a6b983 100644 --- a/enrol/tests/course_enrolment_manager_test.php +++ b/enrol/tests/course_enrolment_manager_test.php @@ -18,6 +18,7 @@ namespace core_enrol; use context_course; use course_enrolment_manager; +use stdClass; /** * Test course_enrolment_manager parts. @@ -557,11 +558,18 @@ class course_enrolment_manager_test extends \advanced_testcase { $this->resetAfterTest(); + // Create the forum. + $record = new stdClass(); + $record->introformat = FORMAT_HTML; + $record->course = $this->course->id; + $forum = self::getDataGenerator()->create_module('forum', $record, ['groupmode' => SEPARATEGROUPS]); + $contextid = $DB->get_field('context', 'id', ['instanceid' => $forum->cmid, 'contextlevel' => CONTEXT_MODULE]); + $teacher = $this->getDataGenerator()->create_and_enrol($this->course, 'teacher'); $this->getDataGenerator()->create_group_member(['groupid' => $this->groups['group1']->id, 'userid' => $teacher->id]); $this->setUser($teacher); - $users = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true); + $courseusers = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true); $this->assertEqualsCanonicalizing([ $teacher->username, $this->users['user0']->username, @@ -570,26 +578,49 @@ class course_enrolment_manager_test extends \advanced_testcase { $this->users['user22']->username, $this->users['userall']->username, $this->users['usertch']->username, - ], array_column($users['users'], 'username')); - $this->assertEquals(7, $users['totalusers']); + ], array_column($courseusers['users'], 'username')); + $this->assertEquals(7, $courseusers['totalusers']); - // Switch course to separate groups. - $this->course->groupmode = SEPARATEGROUPS; - update_course($this->course); - - $users = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true); + $forumusers = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true, $contextid); $this->assertEqualsCanonicalizing([ $teacher->username, $this->users['user1']->username, $this->users['userall']->username, - ], array_column($users['users'], 'username')); - $this->assertEquals(3, $users['totalusers']); + ], array_column($forumusers['users'], 'username')); + $this->assertEquals(3, $forumusers['totalusers']); + + // Switch course to separate groups and forum to no group. + $this->course->groupmode = SEPARATEGROUPS; + update_course($this->course); + set_coursemodule_groupmode($forum->cmid, NOGROUPS); + + $courseusers = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true); + $this->assertEqualsCanonicalizing([ + $teacher->username, + $this->users['user1']->username, + $this->users['userall']->username, + ], array_column($courseusers['users'], 'username')); + $this->assertEquals(3, $courseusers['totalusers']); + + $forumusers = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true, $contextid); + $this->assertEqualsCanonicalizing([ + $teacher->username, + $this->users['user0']->username, + $this->users['user1']->username, + $this->users['user21']->username, + $this->users['user22']->username, + $this->users['userall']->username, + $this->users['usertch']->username, + ], array_column($forumusers['users'], 'username')); + $this->assertEquals(7, $forumusers['totalusers']); + + set_coursemodule_groupmode($forum->cmid, SEPARATEGROUPS); // Allow teacher to access all groups. $roleid = $DB->get_field('role', 'id', ['shortname' => 'teacher']); assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $roleid, context_course::instance($this->course->id)->id); - $users = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true); + $courseusers = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true); $this->assertEqualsCanonicalizing([ $teacher->username, $this->users['user0']->username, @@ -598,7 +629,19 @@ class course_enrolment_manager_test extends \advanced_testcase { $this->users['user22']->username, $this->users['userall']->username, $this->users['usertch']->username, - ], array_column($users['users'], 'username')); - $this->assertEquals(7, $users['totalusers']); + ], array_column($courseusers['users'], 'username')); + $this->assertEquals(7, $courseusers['totalusers']); + + $forumusers = (new course_enrolment_manager($PAGE, $this->course))->search_users('', false, 0, 25, true, $contextid); + $this->assertEqualsCanonicalizing([ + $teacher->username, + $this->users['user0']->username, + $this->users['user1']->username, + $this->users['user21']->username, + $this->users['user22']->username, + $this->users['userall']->username, + $this->users['usertch']->username, + ], array_column($forumusers['users'], 'username')); + $this->assertEquals(7, $forumusers['totalusers']); } } diff --git a/enrol/tests/externallib_test.php b/enrol/tests/externallib_test.php index 868b444ceb2..0a13b95ab5f 100644 --- a/enrol/tests/externallib_test.php +++ b/enrol/tests/externallib_test.php @@ -20,6 +20,7 @@ use core_enrol_external; use core_external\external_api; use enrol_user_enrolment_form; use externallib_advanced_testcase; +use stdClass; defined('MOODLE_INTERNAL') || die(); @@ -1421,6 +1422,71 @@ class externallib_test extends externallib_advanced_testcase { $this->assertCount(0, $result); } + /** + * Test for core_enrol_external::search_users() when group mode is active. + * @covers ::search_users + */ + public function test_search_users_groupmode() { + global $DB; + + $this->resetAfterTest(); + $datagen = $this->getDataGenerator(); + + $studentroleid = $DB->get_field('role', 'id', ['shortname' => 'student'], MUST_EXIST); + $teacherroleid = $DB->get_field('role', 'id', ['shortname' => 'teacher'], MUST_EXIST); + + $course = $datagen->create_course(); + + $student1 = $datagen->create_and_enrol($course); + $student2 = $datagen->create_and_enrol($course); + $student3 = $datagen->create_and_enrol($course); + $teacher1 = $datagen->create_and_enrol($course, 'teacher'); + $teacher2 = $datagen->create_and_enrol($course, 'teacher'); + $teacher3 = $datagen->create_and_enrol($course, 'teacher'); + $teacher4 = $datagen->create_and_enrol($course, 'editingteacher'); + + // Create 2 groups. + $group1 = $datagen->create_group(['courseid' => $course->id]); + $group2 = $datagen->create_group(['courseid' => $course->id]); + + // Add the users to the groups. + $datagen->create_group_member(['groupid' => $group1->id, 'userid' => $student1->id]); + $datagen->create_group_member(['groupid' => $group2->id, 'userid' => $student2->id]); + $datagen->create_group_member(['groupid' => $group2->id, 'userid' => $student3->id]); + $datagen->create_group_member(['groupid' => $group1->id, 'userid' => $teacher1->id]); + $datagen->create_group_member(['groupid' => $group2->id, 'userid' => $teacher1->id]); + $datagen->create_group_member(['groupid' => $group1->id, 'userid' => $teacher2->id]); + + // Create the forum. + $record = new stdClass(); + $record->introformat = FORMAT_HTML; + $record->course = $course->id; + $forum = self::getDataGenerator()->create_module('forum', $record, ['groupmode' => SEPARATEGROUPS]); + $contextid = $DB->get_field('context', 'id', ['instanceid' => $forum->cmid, 'contextlevel' => CONTEXT_MODULE]); + + $this->setUser($teacher1); + $result = core_enrol_external::search_users($course->id, 'user', true, 0, 30, $contextid); + $this->assertCount(5, $result); + + $this->setUser($teacher2); + $result = core_enrol_external::search_users($course->id, 'user', true, 0, 30, $contextid); + $this->assertCount(3, $result); + + $this->setUser($teacher3); + $result = core_enrol_external::search_users($course->id, 'user', true, 0, 30, $contextid); + $this->assertCount(0, $result); + + $this->setUser($teacher4); + $result = core_enrol_external::search_users($course->id, 'user', true, 0, 30, $contextid); + $this->assertCount(7, $result); + + // Now change the group mode to no groups. + set_coursemodule_groupmode($forum->cmid, NOGROUPS); + $this->setUser($teacher1); + $result = core_enrol_external::search_users($course->id, 'user', true, 0, 30, $contextid); + $this->assertCount(7, $result); + } + /** * Tests the get_potential_users external function (not too much detail because the back-end * is covered in another test). diff --git a/enrol/upgrade.txt b/enrol/upgrade.txt index 3c7ffb2d50b..b369e119886 100644 --- a/enrol/upgrade.txt +++ b/enrol/upgrade.txt @@ -3,6 +3,8 @@ information provided here is intended especially for developers. === 4.4 === +* Functions core_enrol_external::search_users and course_enrolment_manager::search_users now have extra optional parameter + contextid which allows to search users in a specific activity context. When omitted it searches in the whole course. * New find_instance() function has been created. It finds a matching enrolment instance in a given course using provided enrolment data (for example cohort idnumber for cohort enrolment). Defaults to null. Override this function in your enrolment plugin if you want it to be supported in CSV course upload. Please be aware that sometimes it is not possible diff --git a/mod/forum/amd/build/form-user-selector.min.js b/mod/forum/amd/build/form-user-selector.min.js index 5a1da302875..19447f7cdb6 100644 --- a/mod/forum/amd/build/form-user-selector.min.js +++ b/mod/forum/amd/build/form-user-selector.min.js @@ -5,6 +5,6 @@ * @copyright 2019 Shamim Rezaie * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -define("mod_forum/form-user-selector",["jquery","core/ajax","core/templates"],(function($,Ajax,Templates){return{processResults:function(selector,results){var users=[];return $.each(results,(function(index,user){users.push({value:user.id,label:user._label})})),users},transport:function(selector,query,success,failure){var courseid=$(selector).attr("courseid");Ajax.call([{methodname:"core_enrol_search_users",args:{courseid:courseid,search:query,searchanywhere:!0,page:0,perpage:30}}])[0].then((function(results){var promises=[],i=0;return $.each(results,(function(index,user){promises.push(Templates.render("mod_forum/form-user-selector-suggestion",user))})),$.when.apply($.when,promises).then((function(){var args=arguments;$.each(results,(function(index,user){user._label=args[i],i++})),success(results)}))})).fail(failure)}}})); +define("mod_forum/form-user-selector",["jquery","core/ajax","core/templates"],(function($,Ajax,Templates){return{processResults:function(selector,results){var users=[];return $.each(results,(function(index,user){users.push({value:user.id,label:user._label})})),users},transport:function(selector,query,success,failure){var courseid=$(selector).attr("courseid"),contextid=$(selector).attr("data-contextid");Ajax.call([{methodname:"core_enrol_search_users",args:{courseid:courseid,search:query,searchanywhere:!0,page:0,perpage:30,contextid:contextid}}])[0].then((function(results){var promises=[],i=0;return $.each(results,(function(index,user){promises.push(Templates.render("mod_forum/form-user-selector-suggestion",user))})),$.when.apply($.when,promises).then((function(){var args=arguments;$.each(results,(function(index,user){user._label=args[i],i++})),success(results)}))})).fail(failure)}}})); //# sourceMappingURL=form-user-selector.min.js.map \ No newline at end of file diff --git a/mod/forum/amd/build/form-user-selector.min.js.map b/mod/forum/amd/build/form-user-selector.min.js.map index 5bf0dae6e27..f97eb3c6230 100644 --- a/mod/forum/amd/build/form-user-selector.min.js.map +++ b/mod/forum/amd/build/form-user-selector.min.js.map @@ -1 +1 @@ -{"version":3,"file":"form-user-selector.min.js","sources":["../src/form-user-selector.js"],"sourcesContent":["// This file is part of Moodle - http://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * Enrolled user selector module.\n *\n * @module mod_forum/form-user-selector\n * @copyright 2019 Shamim Rezaie\n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\n\ndefine(['jquery', 'core/ajax', 'core/templates'], function($, Ajax, Templates) {\n return /** @alias module:mod_forum/form-user-selector */ {\n processResults: function(selector, results) {\n var users = [];\n $.each(results, function(index, user) {\n users.push({\n value: user.id,\n label: user._label\n });\n });\n return users;\n },\n\n transport: function(selector, query, success, failure) {\n var promise;\n var courseid = $(selector).attr('courseid');\n\n promise = Ajax.call([{\n methodname: 'core_enrol_search_users',\n args: {\n courseid: courseid,\n search: query,\n searchanywhere: true,\n page: 0,\n perpage: 30\n }\n }]);\n\n promise[0].then(function(results) {\n var promises = [],\n i = 0;\n\n // Render the label.\n $.each(results, function(index, user) {\n promises.push(Templates.render('mod_forum/form-user-selector-suggestion', user));\n });\n\n // Apply the label to the results.\n return $.when.apply($.when, promises).then(function() {\n var args = arguments;\n $.each(results, function(index, user) {\n user._label = args[i];\n i++;\n });\n success(results);\n return;\n });\n\n }).fail(failure);\n }\n\n };\n\n});\n"],"names":["define","$","Ajax","Templates","processResults","selector","results","users","each","index","user","push","value","id","label","_label","transport","query","success","failure","courseid","attr","call","methodname","args","search","searchanywhere","page","perpage","then","promises","i","render","when","apply","arguments","fail"],"mappings":";;;;;;;AAuBAA,sCAAO,CAAC,SAAU,YAAa,mBAAmB,SAASC,EAAGC,KAAMC,iBACP,CACrDC,eAAgB,SAASC,SAAUC,aAC3BC,MAAQ,UACZN,EAAEO,KAAKF,SAAS,SAASG,MAAOC,MAC5BH,MAAMI,KAAK,CACPC,MAAOF,KAAKG,GACZC,MAAOJ,KAAKK,YAGbR,OAGXS,UAAW,SAASX,SAAUY,MAAOC,QAASC,aAEtCC,SAAWnB,EAAEI,UAAUgB,KAAK,YAEtBnB,KAAKoB,KAAK,CAAC,CACjBC,WAAY,0BACZC,KAAM,CACFJ,SAAUA,SACVK,OAAQR,MACRS,gBAAgB,EAChBC,KAAM,EACNC,QAAS,OAIT,GAAGC,MAAK,SAASvB,aACjBwB,SAAW,GACXC,EAAI,SAGR9B,EAAEO,KAAKF,SAAS,SAASG,MAAOC,MAC5BoB,SAASnB,KAAKR,UAAU6B,OAAO,0CAA2CtB,UAIvET,EAAEgC,KAAKC,MAAMjC,EAAEgC,KAAMH,UAAUD,MAAK,eACnCL,KAAOW,UACXlC,EAAEO,KAAKF,SAAS,SAASG,MAAOC,MAC5BA,KAAKK,OAASS,KAAKO,GACnBA,OAEJb,QAAQZ,eAIb8B,KAAKjB"} \ No newline at end of file +{"version":3,"file":"form-user-selector.min.js","sources":["../src/form-user-selector.js"],"sourcesContent":["// This file is part of Moodle - http://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * Enrolled user selector module.\n *\n * @module mod_forum/form-user-selector\n * @copyright 2019 Shamim Rezaie\n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\n\ndefine(['jquery', 'core/ajax', 'core/templates'], function($, Ajax, Templates) {\n return /** @alias module:mod_forum/form-user-selector */ {\n processResults: function(selector, results) {\n var users = [];\n $.each(results, function(index, user) {\n users.push({\n value: user.id,\n label: user._label\n });\n });\n return users;\n },\n\n transport: function(selector, query, success, failure) {\n var promise;\n var courseid = $(selector).attr('courseid');\n var contextid = $(selector).attr('data-contextid');\n\n promise = Ajax.call([{\n methodname: 'core_enrol_search_users',\n args: {\n courseid: courseid,\n search: query,\n searchanywhere: true,\n page: 0,\n perpage: 30,\n contextid: contextid,\n }\n }]);\n\n promise[0].then(function(results) {\n var promises = [],\n i = 0;\n\n // Render the label.\n $.each(results, function(index, user) {\n promises.push(Templates.render('mod_forum/form-user-selector-suggestion', user));\n });\n\n // Apply the label to the results.\n return $.when.apply($.when, promises).then(function() {\n var args = arguments;\n $.each(results, function(index, user) {\n user._label = args[i];\n i++;\n });\n success(results);\n return;\n });\n\n }).fail(failure);\n }\n\n };\n\n});\n"],"names":["define","$","Ajax","Templates","processResults","selector","results","users","each","index","user","push","value","id","label","_label","transport","query","success","failure","courseid","attr","contextid","call","methodname","args","search","searchanywhere","page","perpage","then","promises","i","render","when","apply","arguments","fail"],"mappings":";;;;;;;AAuBAA,sCAAO,CAAC,SAAU,YAAa,mBAAmB,SAASC,EAAGC,KAAMC,iBACP,CACrDC,eAAgB,SAASC,SAAUC,aAC3BC,MAAQ,UACZN,EAAEO,KAAKF,SAAS,SAASG,MAAOC,MAC5BH,MAAMI,KAAK,CACPC,MAAOF,KAAKG,GACZC,MAAOJ,KAAKK,YAGbR,OAGXS,UAAW,SAASX,SAAUY,MAAOC,QAASC,aAEtCC,SAAWnB,EAAEI,UAAUgB,KAAK,YAC5BC,UAAYrB,EAAEI,UAAUgB,KAAK,kBAEvBnB,KAAKqB,KAAK,CAAC,CACjBC,WAAY,0BACZC,KAAM,CACFL,SAAUA,SACVM,OAAQT,MACRU,gBAAgB,EAChBC,KAAM,EACNC,QAAS,GACTP,UAAWA,cAIX,GAAGQ,MAAK,SAASxB,aACjByB,SAAW,GACXC,EAAI,SAGR/B,EAAEO,KAAKF,SAAS,SAASG,MAAOC,MAC5BqB,SAASpB,KAAKR,UAAU8B,OAAO,0CAA2CvB,UAIvET,EAAEiC,KAAKC,MAAMlC,EAAEiC,KAAMH,UAAUD,MAAK,eACnCL,KAAOW,UACXnC,EAAEO,KAAKF,SAAS,SAASG,MAAOC,MAC5BA,KAAKK,OAASU,KAAKO,GACnBA,OAEJd,QAAQZ,eAIb+B,KAAKlB"} \ No newline at end of file diff --git a/mod/forum/amd/src/form-user-selector.js b/mod/forum/amd/src/form-user-selector.js index 3c69c3210ee..910c847d15d 100644 --- a/mod/forum/amd/src/form-user-selector.js +++ b/mod/forum/amd/src/form-user-selector.js @@ -37,6 +37,7 @@ define(['jquery', 'core/ajax', 'core/templates'], function($, Ajax, Templates) { transport: function(selector, query, success, failure) { var promise; var courseid = $(selector).attr('courseid'); + var contextid = $(selector).attr('data-contextid'); promise = Ajax.call([{ methodname: 'core_enrol_search_users', @@ -45,7 +46,8 @@ define(['jquery', 'core/ajax', 'core/templates'], function($, Ajax, Templates) { search: query, searchanywhere: true, page: 0, - perpage: 30 + perpage: 30, + contextid: contextid, } }]); diff --git a/mod/forum/classes/form/export_form.php b/mod/forum/classes/form/export_form.php index f589e9f4af0..a6936d208df 100644 --- a/mod/forum/classes/form/export_form.php +++ b/mod/forum/classes/form/export_form.php @@ -53,6 +53,7 @@ class export_form extends \moodleform { 'ajax' => 'mod_forum/form-user-selector', 'multiple' => true, 'noselectionstring' => get_string('allusers', 'mod_forum'), + 'data-contextid' => $forum->get_context()->id, 'courseid' => $forum->get_course_id(), 'valuehtmlcallback' => function($value) { global $OUTPUT; @@ -69,7 +70,6 @@ class export_form extends \moodleform { ]; $mform->addElement('autocomplete', 'useridsselected', get_string('users'), [], $options); - // Get the discussions on this forum. $vaultfactory = \mod_forum\local\container::get_vault_factory(); $discussionvault = $vaultfactory->get_discussion_vault(); $discussions = array_map(function($discussion) { diff --git a/mod/forum/classes/local/vaults/discussion.php b/mod/forum/classes/local/vaults/discussion.php index 128c365d2d1..5b93f0596d8 100644 --- a/mod/forum/classes/local/vaults/discussion.php +++ b/mod/forum/classes/local/vaults/discussion.php @@ -26,6 +26,7 @@ namespace mod_forum\local\vaults; defined('MOODLE_INTERNAL') || die(); +use mod_forum\local\container; use mod_forum\local\entities\forum as forum_entity; use mod_forum\local\entities\discussion as discussion_entity; @@ -93,9 +94,23 @@ class discussion extends db_table_vault { * @return array */ public function get_all_discussions_in_forum(forum_entity $forum, string $sort = null): ?array { - $records = $this->get_db()->get_records(self::TABLE, [ - 'forum' => $forum->get_id(), - ], $sort ?? ''); + global $USER; + $options = ['forum' => $forum->get_id()]; + + $managerfactory = container::get_manager_factory(); + $capabilitymanager = $managerfactory->get_capability_manager($forum); + + $select = "forum = :forum"; + + if ($forum->is_in_group_mode() && !$capabilitymanager->can_access_all_groups($USER)) { + $allowedgroups = groups_get_activity_allowed_groups($forum->get_course_module_record()); + $allowedgroups = implode(",", array_keys($allowedgroups)); + if (!$allowedgroups) { + return []; + } + $select .= " AND groupid IN ($allowedgroups)"; + } + $records = $this->get_db()->get_records_select(self::TABLE, $select, $options, $sort ?? ''); return $this->transform_db_records_to_entities($records); } diff --git a/mod/forum/export.php b/mod/forum/export.php index 9d0c48d35b8..e9eb8fef6e1 100644 --- a/mod/forum/export.php +++ b/mod/forum/export.php @@ -192,8 +192,7 @@ if (!$PAGE->has_secondary_navigation()) { // It is possible that the following fields have been provided in the URL. $userids = array_filter($userids, static function(int $userid) use ($course, $cm): bool { - $user = core_user::get_user($userid, '*', MUST_EXIST); - return $cm->effectivegroupmode != SEPARATEGROUPS || user_can_view_profile($user, $course); + return $cm->effectivegroupmode != SEPARATEGROUPS || groups_user_groups_visible($course, $userid, $cm); }); $form->set_data(['useridsselected' => $userids, 'discussionids' => $discussionids, 'from' => $from, 'to' => $to]); diff --git a/mod/forum/tests/behat/forum_export.feature b/mod/forum/tests/behat/forum_export.feature index 2b98f81f16e..05d8ae027a4 100644 --- a/mod/forum/tests/behat/forum_export.feature +++ b/mod/forum/tests/behat/forum_export.feature @@ -48,3 +48,125 @@ Feature: Export forum # This will fail if an exception is thrown. This is the best we can do without the ability to use the download. Hence, there is no "Then" step. And I click on "Export" "button" And I log out + + Scenario: Group mode is respected when exporting discussions + Given the following "groups" exist: + | name | course | idnumber | + | G1 | C1 | G1 | + | G2 | C1 | G2 | + And the following "users" exist: + | username | firstname | lastname | email | + | teachera | Teacher | A | teacherA@example.com | + | teacherb | Teacher | B | teacherB@example.com | + | teacherc | Teacher | C | teacherC@example.com | + | teacherd | Teacher | D | teacherD@example.com | + And the following "course enrolments" exist: + | user | course | role | + | teachera | C1 | teacher | + | teacherb | C1 | teacher | + | teacherc | C1 | teacher | + | teacherd | C1 | teacher | + And the following "group members" exist: + | user | group | + | teachera | G1 | + | teachera | G2 | + | teacherb | G1 | + | teacherc | G2 | + And the following "activities" exist: + | activity | course | idnumber | name | intro | type | section | groupmode | + | forum | C1 | 00001 | Separate groups forum | Standard forum description | general | 1 | 1 | + And the following "mod_forum > discussions" exist: + | user | forum | name | message | group | + | teachera | Separate groups forum | Discussion 1 Group 1 | Test post message | G1 | + | teacherb | Separate groups forum | Discussion 2 Group 1 | Test post message | G1 | + | teachera | Separate groups forum | Discussion 1 Group 2 | Test post message | G2 | + | teacherc | Separate groups forum | Discussion 2 Group 2 | Test post message | G2 | + And I am on the "Separate groups forum" "forum activity" page logged in as teacher1 + And I navigate to "Export" in current page administration + When I expand the "Users" autocomplete + # Editing teacher can see all users and discussions. + Then I should see "Teacher A" in the "Users" "autocomplete" + And I should see "Teacher B" in the "Users" "autocomplete" + And I should see "Teacher C" in the "Users" "autocomplete" + And I should see "Teacher D" in the "Users" "autocomplete" + And I should see "Teacher 1" in the "Users" "autocomplete" + And I should see "Student 1" in the "Users" "autocomplete" + And I press the escape key + And I expand the "Discussions" autocomplete + And I should see "Discussion 1 Group 1" in the "Discussions" "autocomplete" + And I should see "Discussion 2 Group 1" in the "Discussions" "autocomplete" + And I should see "Discussion 1 Group 2" in the "Discussions" "autocomplete" + And I should see "Discussion 2 Group 2" in the "Discussions" "autocomplete" + And I click on "Export" "button" + + And I am on the "Separate groups forum" "forum activity" page logged in as teachera + And I navigate to "Export" in current page administration + When I expand the "Users" autocomplete + # Teacher A is is in both groups. + Then I should see "Teacher A" in the "Users" "autocomplete" + And I should see "Teacher B" in the "Users" "autocomplete" + And I should see "Teacher C" in the "Users" "autocomplete" + And I should not see "Teacher D" in the "Users" "autocomplete" + And I should not see "Teacher 1" in the "Users" "autocomplete" + And I should not see "Student 1" in the "Users" "autocomplete" + And I press the escape key + And I expand the "Discussions" autocomplete + And I should see "Discussion 1 Group 1" in the "Discussions" "autocomplete" + And I should see "Discussion 2 Group 1" in the "Discussions" "autocomplete" + And I should see "Discussion 1 Group 2" in the "Discussions" "autocomplete" + And I should see "Discussion 2 Group 2" in the "Discussions" "autocomplete" + And I click on "Export" "button" + + And I am on the "Separate groups forum" "forum activity" page logged in as teacherb + And I navigate to "Export" in current page administration + When I expand the "Users" autocomplete + # Teacher B is in group 1. + Then I should see "Teacher A" in the "Users" "autocomplete" + And I should see "Teacher B" in the "Users" "autocomplete" + And I should not see "Teacher C" in the "Users" "autocomplete" + And I should not see "Teacher D" in the "Users" "autocomplete" + And I should not see "Teacher 1" in the "Users" "autocomplete" + And I should not see "Student 1" in the "Users" "autocomplete" + And I press the escape key + And I expand the "Discussions" autocomplete + And I should see "Discussion 1 Group 1" in the "Discussions" "autocomplete" + And I should see "Discussion 2 Group 1" in the "Discussions" "autocomplete" + And I should not see "Discussion 1 Group 2" in the "Discussions" "autocomplete" + And I should not see "Discussion 2 Group 2" in the "Discussions" "autocomplete" + And I click on "Export" "button" + + And I am on the "Separate groups forum" "forum activity" page logged in as teacherc + And I navigate to "Export" in current page administration + When I expand the "Users" autocomplete + # Teacher C is in group 2. + Then I should see "Teacher A" in the "Users" "autocomplete" + And I should not see "Teacher B" in the "Users" "autocomplete" + And I should see "Teacher C" in the "Users" "autocomplete" + And I should not see "Teacher D" in the "Users" "autocomplete" + And I should not see "Teacher 1" in the "Users" "autocomplete" + And I should not see "Student 1" in the "Users" "autocomplete" + And I press the escape key + And I expand the "Discussions" autocomplete + And I should not see "Discussion 1 Group 1" in the "Discussions" "autocomplete" + And I should not see "Discussion 2 Group 1" in the "Discussions" "autocomplete" + And I should see "Discussion 1 Group 2" in the "Discussions" "autocomplete" + And I should see "Discussion 2 Group 2" in the "Discussions" "autocomplete" + And I click on "Export" "button" + + And I am on the "Separate groups forum" "forum activity" page logged in as teacherd + And I navigate to "Export" in current page administration + When I expand the "Users" autocomplete + # Teacher D is in no group. + Then I should not see "Teacher A" in the "Users" "autocomplete" + And I should not see "Teacher B" in the "Users" "autocomplete" + And I should not see "Teacher C" in the "Users" "autocomplete" + And I should not see "Teacher D" in the "Users" "autocomplete" + And I should not see "Teacher 1" in the "Users" "autocomplete" + And I should not see "Student 1" in the "Users" "autocomplete" + And I press the escape key + And I expand the "Discussions" autocomplete + And I should not see "Discussion 1 Group 1" in the "Discussions" "autocomplete" + And I should not see "Discussion 2 Group 1" in the "Discussions" "autocomplete" + And I should not see "Discussion 1 Group 2" in the "Discussions" "autocomplete" + And I should not see "Discussion 2 Group 2" in the "Discussions" "autocomplete" + And I click on "Export" "button"