L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-71261 mod_quiz: Quiz user override should only get enrolled users

Browse Source 
Also update similar code in mod_assign to use the improved APIs.

Co-Authored-By: Tim Hunt <[email protected]>
This commit is contained in:
Rex Lorenzo
2022-12-12 12:29:19 +00:00
committed by Tim Hunt
co-authored by Tim Hunt
parent 40a89d8a9a
commit ebeaa4b3f2
7 changed files with 144 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/enrollib.php
+13 -13
View File
@@ -1453,13 +1453,13 @@ function is_enrolled(context $context, $user = null, $withcapability = '', $only
* @param string|array $capability optional, may include a capability name, or array of names.
* If an array is provided then this is the equivalent of a logical 'OR',
* i.e. the user needs to have one of these capabilities.
* @param int $group optional, 0 indicates no current group and USERSWITHOUTGROUP users without any group; otherwise the group id
* @param int|array $groupids The groupids, 0 or [] means all groups and USERSWITHOUTGROUP no group
* @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions
* @param bool $onlysuspended inverse of onlyactive, consider only suspended enrolments
* @param int $enrolid The enrolment ID. If not 0, only users enrolled using this enrolment method will be returned.
* @return \core\dml\sql_join Contains joins, wheres, params and cannotmatchanyrows
*/
function get_enrolled_with_capabilities_join(context $context, $prefix = '', $capability = '', $group = 0,
function get_enrolled_with_capabilities_join(context $context, $prefix = '', $capability = '', $groupids = 0,
$onlyactive = false, $onlysuspended = false, $enrolid = 0) {
$uid = $prefix . 'u.id';
$joins = array();
@@ -1480,8 +1480,8 @@ function get_enrolled_with_capabilities_join(context $context, $prefix = '', $ca
$cannotmatchanyrows = $cannotmatchanyrows || $capjoin->cannotmatchanyrows;
}
if ($group) {
$groupjoin = groups_get_members_join($group, $uid, $context);
if ($groupids) {
$groupjoin = groups_get_members_join($groupids, $uid, $context);
$joins[] = $groupjoin->joins;
$params = array_merge($params, $groupjoin->params);
if (!empty($groupjoin->wheres)) {
@@ -1505,13 +1505,13 @@ function get_enrolled_with_capabilities_join(context $context, $prefix = '', $ca
*
* @param context $context
* @param string $withcapability
* @param int $groupid 0 means ignore groups, USERSWITHOUTGROUP without any group and any other value limits the result by group id
* @param int|array $groupids The groupids, 0 or [] means all groups and USERSWITHOUTGROUP no group
* @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions
* @param bool $onlysuspended inverse of onlyactive, consider only suspended enrolments
* @param int $enrolid The enrolment ID. If not 0, only users enrolled using this enrolment method will be returned.
* @return array list($sql, $params)
*/
function get_enrolled_sql(context $context, $withcapability = '', $groupid = 0, $onlyactive = false, $onlysuspended = false,
function get_enrolled_sql(context $context, $withcapability = '', $groupids = 0, $onlyactive = false, $onlysuspended = false,
$enrolid = 0) {
// Use unique prefix just in case somebody makes some SQL magic with the result.
@@ -1520,7 +1520,7 @@ function get_enrolled_sql(context $context, $withcapability = '', $groupid = 0,
$prefix = 'eu' . $i . '_';
$capjoin = get_enrolled_with_capabilities_join(
$context, $prefix, $withcapability, $groupid, $onlyactive, $onlysuspended, $enrolid);
$context, $prefix, $withcapability, $groupids, $onlyactive, $onlysuspended, $enrolid);
$sql = "SELECT DISTINCT {$prefix}u.id
FROM {user} {$prefix}u
@@ -1631,7 +1631,7 @@ function get_enrolled_join(context $context, $useridcolumn, $onlyactive = false,
*
* @param context $context
* @param string $withcapability
* @param int $groupid 0 means ignore groups, USERSWITHOUTGROUP without any group and any other value limits the result by group id
* @param int|array $groupids The groupids, 0 or [] means all groups and USERSWITHOUTGROUP no group
* @param string $userfields requested user record fields
* @param string $orderby
* @param int $limitfrom return a subset of records, starting at this point (optional, required if $limitnum is set).
@@ -1639,11 +1639,11 @@ function get_enrolled_join(context $context, $useridcolumn, $onlyactive = false,
* @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions
* @return array of user records
*/
function get_enrolled_users(context $context, $withcapability = '', $groupid = 0, $userfields = 'u.*', $orderby = null,
function get_enrolled_users(context $context, $withcapability = '', $groupids = 0, $userfields = 'u.*', $orderby = null,
$limitfrom = 0, $limitnum = 0, $onlyactive = false) {
global $DB;
list($esql, $params) = get_enrolled_sql($context, $withcapability, $groupid, $onlyactive);
list($esql, $params) = get_enrolled_sql($context, $withcapability, $groupids, $onlyactive);
$sql = "SELECT $userfields
FROM {user} u
JOIN ($esql) je ON je.id = u.id
@@ -1665,15 +1665,15 @@ function get_enrolled_users(context $context, $withcapability = '', $groupid = 0
*
* @param context $context
* @param string $withcapability
* @param int $groupid 0 means ignore groups, any other value limits the result by group id
* @param int|array $groupids The groupids, 0 or [] means all groups and USERSWITHOUTGROUP no group
* @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions
* @return int number of users enrolled into course
*/
function count_enrolled_users(context $context, $withcapability = '', $groupid = 0, $onlyactive = false) {
function count_enrolled_users(context $context, $withcapability = '', $groupids = 0, $onlyactive = false) {
global $DB;
$capjoin = get_enrolled_with_capabilities_join(
$context, '', $withcapability, $groupid, $onlyactive);
$context, '', $withcapability, $groupids, $onlyactive);
$sql = "SELECT COUNT(DISTINCT u.id)
FROM {user} u
lib/tests/accesslib_test.php
+73 -1
View File
@@ -2745,7 +2745,13 @@ class accesslib_test extends advanced_testcase {
* Test that enrolled users SQL does not return any values for users in
* other courses.
*
*
* @covers ::get_enrolled_users
* @covers ::get_enrolled_sql
* @covers ::get_enrolled_with_capabilities_join
* @covers ::get_enrolled_join
* @covers ::get_with_capability_join
* @covers ::groups_get_members_join
* @covers ::get_suspended_userids
*/
public function test_get_enrolled_sql_different_course() {
@@ -2778,7 +2784,13 @@ class accesslib_test extends advanced_testcase {
* Test that enrolled users SQL does not return any values for role
* assignments without an enrolment.
*
*
* @covers ::get_enrolled_users
* @covers ::get_enrolled_sql
* @covers ::get_enrolled_with_capabilities_join
* @covers ::get_enrolled_join
* @covers ::get_with_capability_join
* @covers ::groups_get_members_join
* @covers ::get_suspended_userids
*/
public function test_get_enrolled_sql_role_only() {
@@ -2810,6 +2822,11 @@ class accesslib_test extends advanced_testcase {
* Test that multiple enrolments for the same user are counted correctly.
*
* @covers ::get_enrolled_users
* @covers ::get_enrolled_sql
* @covers ::get_enrolled_with_capabilities_join
* @covers ::get_enrolled_join
* @covers ::get_with_capability_join
* @covers ::groups_get_members_join
* @covers ::get_suspended_userids
*/
public function test_get_enrolled_sql_multiple_enrolments() {
@@ -2857,11 +2874,66 @@ class accesslib_test extends advanced_testcase {
}
/**
* Test that enrolled users returns only users in those groups that are
* specified.
*
* @covers ::get_enrolled_users
* @covers ::get_enrolled_sql
* @covers ::get_enrolled_with_capabilities_join
* @covers ::get_enrolled_join
* @covers ::get_with_capability_join
* @covers ::groups_get_members_join
* @covers ::get_suspended_userids
*/
public function test_get_enrolled_sql_userswithgroups() {
$this->resetAfterTest();
$systemcontext = context_system::instance();
$course = $this->getDataGenerator()->create_course();
$coursecontext = context_course::instance($course->id);
$user1 = $this->getDataGenerator()->create_user();
$user2 = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($user1->id, $course->id);
$this->getDataGenerator()->enrol_user($user2->id, $course->id);
$group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]);
groups_add_member($group1, $user1);
$group2 = $this->getDataGenerator()->create_group(['courseid' => $course->id]);
groups_add_member($group2, $user2);
// Get user from group 1.
$group1users = get_enrolled_users($coursecontext, '', $group1->id);
$this->assertCount(1, $group1users);
$this->assertArrayHasKey($user1->id, $group1users);
$this->assertEquals(1, count_enrolled_users($coursecontext, '', $group1->id));
// Get user from group 2.
$group2users = get_enrolled_users($coursecontext, '', $group2->id);
$this->assertCount(1, $group2users);
$this->assertArrayHasKey($user2->id, $group2users);
$this->assertEquals(1, count_enrolled_users($coursecontext, '', $group2->id));
// Get users from multiple groups.
$groupusers = get_enrolled_users($coursecontext, '', [$group1->id, $group2->id]);
$this->assertCount(2, $groupusers);
$this->assertArrayHasKey($user1->id, $groupusers);
$this->assertArrayHasKey($user2->id, $groupusers);
$this->assertEquals(2, count_enrolled_users($coursecontext, '', [$group1->id, $group2->id]));
}
/**
* Test that enrolled users SQL does not return any values for users
* without a group when $context is not a valid course context.
*
* @covers ::get_enrolled_users
* @covers ::get_enrolled_sql
* @covers ::get_enrolled_with_capabilities_join
* @covers ::get_enrolled_join
* @covers ::get_with_capability_join
* @covers ::groups_get_members_join
* @covers ::get_suspended_userids
*/
public function test_get_enrolled_sql_userswithoutgroup() {
global $DB;
@@ -2880,7 +2952,7 @@ class accesslib_test extends advanced_testcase {
$group = $this->getDataGenerator()->create_group(array('courseid' => $course->id));
groups_add_member($group, $user1);
$enrolled = get_enrolled_users($coursecontext);
$enrolled = get_enrolled_users($coursecontext);
$this->assertCount(2, $enrolled);
// Get users without any group on the course context.
lib/upgrade.txt
+11
View File
@@ -1,5 +1,15 @@
This files describes API changes in core libraries and APIs,
information provided here is intended especially for developers.
=== 4.2 ===
* In enrollib.php, the methods get_enrolled_with_capabilities_join, get_enrolled_sql, get_enrolled_users and
count_enrolled_users used to only be able to accept a single group id, even though internally, they used
groups_get_members_join which could also accept an array of groups, or the constant USERSWITHOUTGROUP.
This has now been made consistent. These enrol methods now accept all the group-related options that
groups_get_members_join can handle.
=== 4.1 ===
* HTMLPurifier has been upgraded to the latest version - 4.16.0
@@ -696,6 +706,7 @@ filepath because some components, such as mod_page or mod_resource, add the revi
call the above function because the file is sent by a third party library, then you should add
the attribute data-double-submit-protection="off" to your form.
=== 3.7 ===
* Nodes in the navigation api can have labels for each group. See set/get_collectionlabel().
mod/assign/override_form.php
+7 -16
View File
@@ -157,23 +157,14 @@ class assign_override_form extends moodleform {
// Get the list of appropriate users, depending on whether and how groups are used.
$userfieldsapi = \core_user\fields::for_name();
if ($accessallgroups) {
$users = get_enrolled_users($this->context, '', 0,
'u.id, u.email, ' . $userfieldsapi->get_sql('u', false, '', '', false)->selects, $sort);
} else if ($groups = groups_get_activity_allowed_groups($cm)) {
$enrolledjoin = get_enrolled_join($this->context, 'u.id');
$userfields = 'u.id, u.email, ' . $userfieldsapi->get_sql('u', false, '', '', false)->selects;
list($ingroupsql, $ingroupparams) = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED);
$params = $enrolledjoin->params + $ingroupparams;
$sql = "SELECT $userfields
FROM {user} u
JOIN {groups_members} gm ON gm.userid = u.id
{$enrolledjoin->joins}
WHERE gm.groupid $ingroupsql
AND {$enrolledjoin->wheres}
ORDER BY $sort";
$users = $DB->get_records_sql($sql, $params);
$userfields = 'u.id, u.email, ' . $userfieldsapi->get_sql('u', false, '', '', false)->selects;
$groupids = 0;
if (!$accessallgroups) {
$groups = groups_get_activity_allowed_groups($cm);
$groupids = array_keys($groups);
}
$users = get_enrolled_users($this->context, '',
$groupids, $userfields, $sort);
// Filter users based on any fixed restrictions (groups, profile).
$info = new \core_availability\info_module($cm);
mod/lesson/override_form.php
+7 -16
View File
@@ -141,23 +141,14 @@ class lesson_override_form extends moodleform {
// Get the list of appropriate users, depending on whether and how groups are used.
$userfieldsapi = \core_user\fields::for_name();
if ($accessallgroups) {
$users = get_enrolled_users($this->context, '', 0,
'u.id, u.email, ' . $userfieldsapi->get_sql('u', false, '', '', false)->selects, $sort);
} else if ($groups = groups_get_activity_allowed_groups($cm)) {
$enrolledjoin = get_enrolled_join($this->context, 'u.id');
$userfields = 'u.id, u.email, ' . $userfieldsapi->get_sql('u', false, '', '', false)->selects;
list($ingroupsql, $ingroupparams) = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED);
$params = $enrolledjoin->params + $ingroupparams;
$sql = "SELECT $userfields
FROM {user} u
JOIN {groups_members} gm ON gm.userid = u.id
{$enrolledjoin->joins}
WHERE gm.groupid $ingroupsql
AND {$enrolledjoin->wheres}
ORDER BY $sort";
$users = $DB->get_records_sql($sql, $params);
$userfields = 'u.id, u.email, ' . $userfieldsapi->get_sql('u', false, '', '', false)->selects;
$groupids = 0;
if (!$accessallgroups) {
$groups = groups_get_activity_allowed_groups($cm);
$groupids = array_keys($groups);
}
$users = get_enrolled_users($this->context, '',
$groupids, $userfields, $sort);
// Filter users based on any fixed restrictions (groups, profile).
$info = new \core_availability\info_module($cm);
mod/quiz/override_form.php
+11 -26
View File
@@ -135,43 +135,28 @@ class quiz_override_form extends moodleform {
$mform->freeze('userid');
} else {
// Prepare the list of users.
// Get the list of appropriate users, depending on whether and how groups are used.
$userfieldsql = $userfieldsapi->get_sql('u', true, '', '', false);
$capabilityjoin = get_with_capability_join($this->context, 'mod/quiz:attempt', 'u.id');
list($sort, $sortparams) = users_order_by_sql('u', null,
$this->context, $userfieldsql->mappings);
$groupjoin = '';
$groupparams = [];
$groupids = 0;
if (!$accessallgroups) {
$groups = groups_get_activity_allowed_groups($cm);
list($grouptest, $groupparams) = $DB->get_in_or_equal(
array_keys($groups), SQL_PARAMS_NAMED, 'grp');
$groupjoin = "JOIN (SELECT DISTINCT userid
FROM {groups_members}
WHERE groupid $grouptest
) gm ON gm.userid = u.id";
}
$capabilitywhere = '';
if ($capabilityjoin->wheres) {
$capabilitywhere = 'AND ' . $capabilityjoin->wheres;
$groupids = array_keys($groups);
}
$enrolledjoin = get_enrolled_with_capabilities_join(
$this->context, '', 'mod/quiz:attempt', $groupids, true);
$userfieldsql = $userfieldsapi->get_sql('u', true, '', '', false);
list($sort, $sortparams) = users_order_by_sql('u', null,
$this->context, $userfieldsql->mappings);
$users = $DB->get_records_sql("
SELECT $userfieldsql->selects
FROM {user} u
$enrolledjoin->joins
$userfieldsql->joins
LEFT JOIN {quiz_overrides} existingoverride ON
existingoverride.userid = u.id AND existingoverride.quiz = :quizid
$capabilityjoin->joins
$groupjoin
$userfieldsql->joins
WHERE existingoverride.id IS NULL
$capabilitywhere
AND $enrolledjoin->wheres
ORDER BY $sort
", array_merge(['quizid' => $this->quiz->id], $capabilityjoin->params,
$groupparams, $userfieldsql->params, $sortparams));
", array_merge(['quizid' => $this->quiz->id], $userfieldsql->params, $enrolledjoin->params, $sortparams));
// Filter users based on any fixed restrictions (groups, profile).
$info = new \core_availability\info_module($cm);
mod/quiz/tests/behat/quiz_user_override.feature
+22
View File
@@ -196,3 +196,25 @@ Feature: Quiz user override
| quiz | Other quiz | C2 | quiz2 |
When I am on the "Other quiz" "mod_quiz > User overrides" page logged in as "admin"
Then the "Add user override" "button" should be disabled
@javascript
Scenario: Should see only enrolled users in user selector
Given the following "users" exist:
| username | firstname | lastname | email |
| manager | Max | Manager | man@example.com |
And the following "role assigns" exist:
| user | role | contextlevel | reference |
| manager | manager | System | |
And the following "activities" exist:
| activity | name | course | idnumber | groupmode |
| quiz | Test quiz | C1 | quiz1 | 1 |
And I log in as "admin"
And I set the following system permissions of "Manager" role:
| capability | permission |
| mod/quiz:attempt | Allow |
And I log out
When I am on the "Test quiz" "mod_quiz > User overrides" page logged in as "teacher"
And I press "Add user override"
And I click on "Override user" "field"
And I type "Max Manager"
Then I should see "No suggestions"