Skip to content

Commit

Permalink
MDL-80504 forum: Fix seperate group mode
Browse files Browse the repository at this point in the history
  • Loading branch information
@ilyatregubov @andrewnicols
ilyatregubov authored and andrewnicols committed Feb 8, 2024
1 parent fba3e87 commit 1c059cb
Show file tree
Hide file tree
Showing 12 changed files with 306 additions and 32 deletions.
29 changes: 20 additions & 9 deletions enrol/externallib.php
View file
Expand Up @@ -629,6 +629,7 @@ public static function search_users_parameters(): external_function_parameters {
'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),
]
);
}
Expand All @@ -641,11 +642,12 @@ public static function search_users_parameters(): external_function_parameters {
* @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');
Expand All @@ -657,10 +659,15 @@ public static function search_users(int $courseid, string $search, bool $searcha
'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) {
Expand All @@ -674,10 +681,14 @@ public static function search_users(int $courseid, string $search, bool $searcha
$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.
Expand Down
22 changes: 18 additions & 4 deletions enrol/locallib.php
View file
Expand Up @@ -563,26 +563,40 @@ public function search_other_users($search = '', $searchanywhere = false, $page
* @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)';
Expand Down
63 changes: 53 additions & 10 deletions enrol/tests/course_enrolment_manager_test.php
View file
Expand Up @@ -18,6 +18,7 @@

use context_course;
use course_enrolment_manager;
use stdClass;

/**
* Test course_enrolment_manager parts.
Expand Down Expand Up @@ -557,11 +558,18 @@ public function test_search_users_course_groupmode(): void {

$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,
Expand All @@ -570,26 +578,61 @@ public function test_search_users_course_groupmode(): void {
$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.
$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($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);

$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['user1']->username,
$this->users['userall']->username,
], array_column($users['users'], 'username'));
$this->assertEquals(3, $users['totalusers']);
], 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,
$this->users['user1']->username,
$this->users['user21']->username,
$this->users['user22']->username,
$this->users['userall']->username,
$this->users['usertch']->username,
], 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,
Expand All @@ -598,7 +641,7 @@ public function test_search_users_course_groupmode(): void {
$this->users['user22']->username,
$this->users['userall']->username,
$this->users['usertch']->username,
], array_column($users['users'], 'username'));
$this->assertEquals(7, $users['totalusers']);
], array_column($forumusers['users'], 'username'));
$this->assertEquals(7, $forumusers['totalusers']);
}
}
66 changes: 66 additions & 0 deletions enrol/tests/externallib_test.php
View file
Expand Up @@ -20,6 +20,7 @@
use core_external\external_api;
use enrol_user_enrolment_form;
use externallib_advanced_testcase;
use stdClass;

defined('MOODLE_INTERNAL') || die();

Expand Down Expand Up @@ -1421,6 +1422,71 @@ public function test_search_users() {
$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).
Expand Down
2 changes: 2 additions & 0 deletions enrol/upgrade.txt
View file
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion mod/forum/amd/build/form-user-selector.min.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 1c059cb

Please sign in to comment.