From e53bc6a1f805ac9a6a8b066df12e4f9e06ee0838 Mon Sep 17 00:00:00 2001 From: Ilya Tregubov Date: Mon, 23 Nov 2020 11:23:49 +0200 Subject: [PATCH 1/3] MDL-69869 enrol: Add ability to get suspended users through core_enrol_get_enrolled_users webservice. --- enrol/externallib.php | 17 ++++-- enrol/tests/externallib_test.php | 89 ++++++++++++++++++++++++++++++++ enrol/upgrade.txt | 4 ++ 3 files changed, 107 insertions(+), 3 deletions(-) diff --git a/enrol/externallib.php b/enrol/externallib.php index d0fbaa9453a..ff623ca8bad 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -693,7 +693,13 @@ class core_enrol_external extends external_api { isn\'t defined, returns all the viewable users. This option requires \'moodle/site:accessallgroups\' on the course context if the user doesn\'t belong to the group. - * onlyactive (integer) return only users with active enrolments and matching time restrictions. This option requires \'moodle/course:enrolreview\' on the course context. + * onlyactive (integer) return only users with active enrolments and matching time restrictions. + This option requires \'moodle/course:enrolreview\' on the course context. + Please note that this option can\'t + be used together with onlysuspended (only one can be active). + * onlysuspended (integer) return only suspended users. This option requires + \'moodle/course:enrolreview\' on the course context. Please note that this option can\'t + be used together with onlyactive (only one can be active). * userfields (\'string, string, ...\') return only the values of these user fields. * limitfrom (integer) sql limit from. * limitnumber (integer) maximum number of returned users. @@ -730,6 +736,7 @@ class core_enrol_external extends external_api { $withcapability = ''; $groupid = 0; $onlyactive = false; + $onlysuspended = false; $userfields = array(); $limitfrom = 0; $limitnumber = 0; @@ -747,6 +754,9 @@ class core_enrol_external extends external_api { case 'onlyactive': $onlyactive = !empty($option['value']); break; + case 'onlysuspended': + $onlysuspended = !empty($option['value']); + break; case 'userfields': $thefields = explode(',', $option['value']); foreach ($thefields as $f) { @@ -809,11 +819,12 @@ class core_enrol_external extends external_api { require_capability('moodle/site:accessallgroups', $coursecontext); } // to overwrite this option, you need course:enrolereview permission - if ($onlyactive) { + if ($onlyactive || $onlysuspended) { require_capability('moodle/course:enrolreview', $coursecontext); } - list($enrolledsql, $enrolledparams) = get_enrolled_sql($coursecontext, $withcapability, $groupid, $onlyactive); + list($enrolledsql, $enrolledparams) = get_enrolled_sql($coursecontext, $withcapability, $groupid, $onlyactive, + $onlysuspended); $ctxselect = ', ' . context_helper::get_preload_record_columns_sql('ctx'); $ctxjoin = "LEFT JOIN {context} ctx ON (ctx.instanceid = u.id AND ctx.contextlevel = :contextlevel)"; $enrolledparams['contextlevel'] = CONTEXT_USER; diff --git a/enrol/tests/externallib_test.php b/enrol/tests/externallib_test.php index a12df0bb38b..e80fb3dbdb4 100644 --- a/enrol/tests/externallib_test.php +++ b/enrol/tests/externallib_test.php @@ -355,6 +355,95 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase { } } + /** + * Verify get_enrolled_users() returned users according to their status. + */ + public function test_get_enrolled_users_active_suspended() { + global $USER; + + $this->resetAfterTest(); + + // Create the course and the users. + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + $user0 = $this->getDataGenerator()->create_user(array('username' => 'user0active')); + $user1 = $this->getDataGenerator()->create_user(array('username' => 'user1active')); + $user2 = $this->getDataGenerator()->create_user(array('username' => 'user2active')); + $user2su = $this->getDataGenerator()->create_user(array('username' => 'user2suspended')); // Suspended user. + $user3 = $this->getDataGenerator()->create_user(array('username' => 'user3active')); + $user3su = $this->getDataGenerator()->create_user(array('username' => 'user3suspended')); // Suspended user. + + // Enrol the users in the course. + $this->getDataGenerator()->enrol_user($user0->id, $course->id); + $this->getDataGenerator()->enrol_user($user1->id, $course->id); + $this->getDataGenerator()->enrol_user($user2->id, $course->id); + $this->getDataGenerator()->enrol_user($user2su->id, $course->id, null, 'manual', 0, 0, ENROL_USER_SUSPENDED); + $this->getDataGenerator()->enrol_user($user3->id, $course->id); + $this->getDataGenerator()->enrol_user($user3su->id, $course->id, null, 'manual', 0, 0, ENROL_USER_SUSPENDED); + + // Create a role to add the allowedcaps. Users will have this role assigned. + $roleid = $this->getDataGenerator()->create_role(); + // Allow the specified capabilities. + assign_capability('moodle/course:enrolreview', CAP_ALLOW, $roleid, $coursecontext); + assign_capability('moodle/user:viewalldetails', CAP_ALLOW, $roleid, $coursecontext); + + // Switch to the user and assign the role. + $this->setUser($user0); + role_assign($roleid, $USER->id, $coursecontext); + + // Suspended users. + $options = array( + array('name' => 'onlysuspended', 'value' => true), + array('name' => 'userfields', 'value' => 'id,username') + ); + $suspendedusers = core_enrol_external::get_enrolled_users($course->id, $options); + + // We need to execute the return values cleaning process to simulate the web service server. + $suspendedusers = external_api::clean_returnvalue(core_enrol_external::get_enrolled_users_returns(), $suspendedusers); + $this->assertCount(2, $suspendedusers); + + foreach ($suspendedusers as $suspendeduser) { + $this->assertStringContainsString('suspended', $suspendeduser['username']); + } + + // Active users. + $options = array( + array('name' => 'onlyactive', 'value' => true), + array('name' => 'userfields', 'value' => 'id,username') + ); + $activeusers = core_enrol_external::get_enrolled_users($course->id, $options); + + // We need to execute the return values cleaning process to simulate the web service server. + $activeusers = external_api::clean_returnvalue(core_enrol_external::get_enrolled_users_returns(), $activeusers); + $this->assertCount(4, $activeusers); + + foreach ($activeusers as $activeuser) { + $this->assertStringContainsString('active', $activeuser['username']); + } + + // All enrolled users. + $options = array( + array('name' => 'userfields', 'value' => 'id,username') + ); + $allusers = core_enrol_external::get_enrolled_users($course->id, $options); + + // We need to execute the return values cleaning process to simulate the web service server. + $allusers = external_api::clean_returnvalue(core_enrol_external::get_enrolled_users_returns(), $allusers); + $this->assertCount(6, $allusers); + + // Active and suspended. Test exception is thrown. + $options = [ + ['name' => 'onlyactive', 'value' => true], + ['name' => 'onlysuspended', 'value' => true], + ['name' => 'userfields', 'value' => 'id,username'] + ]; + $this->expectException('coding_exception'); + $message = 'Coding error detected, it must be fixed by a programmer: Both onlyactive ' . + 'and onlysuspended are set, this is probably not what you want!'; + $this->expectExceptionMessage($message); + core_enrol_external::get_enrolled_users($course->id, $options); + } + /** * Test get_users_courses */ diff --git a/enrol/upgrade.txt b/enrol/upgrade.txt index bfec2ff74c7..a882f6ee067 100644 --- a/enrol/upgrade.txt +++ b/enrol/upgrade.txt @@ -1,6 +1,10 @@ This files describes API changes in /enrol/* - plugins, information provided here is intended especially for developers. +=== 3.11 === + +* Added onlysuspended option to core_enrol_get_enrolled_users webservice to retrieve only suspended users. + === 3.8 === * Function enrol_manual_plugin::enrol_cohort now return the number of enrolled cohort users. From 65a1d6e8e06836e3549ffe44bf94d4d276421d01 Mon Sep 17 00:00:00 2001 From: Ilya Tregubov Date: Fri, 27 Nov 2020 09:59:46 +0200 Subject: [PATCH 2/3] MDL-69869 enrol: Clean up - switch to [] from array(). --- enrol/externallib.php | 54 ++++++++++++++++---------------- enrol/tests/externallib_test.php | 34 ++++++++++---------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/enrol/externallib.php b/enrol/externallib.php index ff623ca8bad..99988ad099f 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -679,14 +679,14 @@ class core_enrol_external extends external_api { */ public static function get_enrolled_users_parameters() { return new external_function_parameters( - array( + [ 'courseid' => new external_value(PARAM_INT, 'course id'), 'options' => new external_multiple_structure( new external_single_structure( - array( + [ 'name' => new external_value(PARAM_ALPHANUMEXT, 'option name'), 'value' => new external_value(PARAM_RAW, 'option value') - ) + ] ), 'Option names: * withcapability (string) return only users with this capability. This option requires \'moodle/role:review\' on the course context. * groupid (integer) return only users in this group id. If the course has groups enabled and this param @@ -705,8 +705,8 @@ class core_enrol_external extends external_api { * limitnumber (integer) maximum number of returned users. * sortby (string) sort by id, firstname or lastname. For ordering like the site does, use siteorder. * sortdirection (string) ASC or DESC', - VALUE_DEFAULT, array()), - ) + VALUE_DEFAULT, []), + ] ); } @@ -720,7 +720,7 @@ class core_enrol_external extends external_api { * } * @return array An array of users */ - public static function get_enrolled_users($courseid, $options = array()) { + public static function get_enrolled_users($courseid, $options = []) { global $CFG, $USER, $DB; require_once($CFG->dirroot . '/course/lib.php'); @@ -728,20 +728,20 @@ class core_enrol_external extends external_api { $params = self::validate_parameters( self::get_enrolled_users_parameters(), - array( + [ 'courseid'=>$courseid, 'options'=>$options - ) + ] ); $withcapability = ''; $groupid = 0; $onlyactive = false; $onlysuspended = false; - $userfields = array(); + $userfields = []; $limitfrom = 0; $limitnumber = 0; $sortby = 'us.id'; - $sortparams = array(); + $sortparams = []; $sortdirection = 'ASC'; foreach ($options as $option) { switch ($option['name']) { @@ -770,7 +770,7 @@ class core_enrol_external extends external_api { $limitnumber = clean_param($option['value'], PARAM_INT); break; case 'sortby': - $sortallowedvalues = array('id', 'firstname', 'lastname', 'siteorder'); + $sortallowedvalues = ['id', 'firstname', 'lastname', 'siteorder']; if (!in_array($option['value'], $sortallowedvalues)) { throw new invalid_parameter_exception('Invalid value for sortby parameter (value: ' . $option['value'] . '),' . 'allowed values are: ' . implode(',', $sortallowedvalues)); @@ -783,7 +783,7 @@ class core_enrol_external extends external_api { break; case 'sortdirection': $sortdirection = strtoupper($option['value']); - $directionallowedvalues = array('ASC', 'DESC'); + $directionallowedvalues = ['ASC', 'DESC']; if (!in_array($sortdirection, $directionallowedvalues)) { throw new invalid_parameter_exception('Invalid value for sortdirection parameter (value: ' . $sortdirection . '),' . 'allowed values are: ' . implode(',', $directionallowedvalues)); @@ -792,7 +792,7 @@ class core_enrol_external extends external_api { } } - $course = $DB->get_record('course', array('id'=>$courseid), '*', MUST_EXIST); + $course = $DB->get_record('course', ['id' => $courseid], '*', MUST_EXIST); $coursecontext = context_course::instance($courseid, IGNORE_MISSING); if ($courseid == SITEID) { $context = context_system::instance(); @@ -840,7 +840,7 @@ class core_enrol_external extends external_api { $enrolledparams = array_merge($enrolledparams, $groupparams); } else { // User doesn't belong to any group, so he can't see any user. Return an empty array. - return array(); + return []; } } $sql = "SELECT us.*, COALESCE(ul.timeaccess, 0) AS lastcourseaccess @@ -856,7 +856,7 @@ class core_enrol_external extends external_api { $enrolledparams['courseid'] = $courseid; $enrolledusers = $DB->get_recordset_sql($sql, $enrolledparams, $limitfrom, $limitnumber); - $users = array(); + $users = []; foreach ($enrolledusers as $user) { context_helper::preload_from_record($user); if ($userdetails = user_get_user_details($user, $course, $userfields)) { @@ -876,7 +876,7 @@ class core_enrol_external extends external_api { public static function get_enrolled_users_returns() { return new external_multiple_structure( new external_single_structure( - array( + [ 'id' => new external_value(PARAM_INT, 'ID of the user'), 'username' => new external_value(PARAM_RAW, 'Username policy is defined in Moodle security config', VALUE_OPTIONAL), 'firstname' => new external_value(PARAM_NOTAGS, 'The first name(s) of the user', VALUE_OPTIONAL), @@ -907,47 +907,47 @@ class core_enrol_external extends external_api { 'profileimageurl' => new external_value(PARAM_URL, 'User image profile URL - big version', VALUE_OPTIONAL), 'customfields' => new external_multiple_structure( new external_single_structure( - array( + [ 'type' => new external_value(PARAM_ALPHANUMEXT, 'The type of the custom field - text field, checkbox...'), 'value' => new external_value(PARAM_RAW, 'The value of the custom field'), 'name' => new external_value(PARAM_RAW, 'The name of the custom field'), 'shortname' => new external_value(PARAM_RAW, 'The shortname of the custom field - to be able to build the field class in the code'), - ) + ] ), 'User custom fields (also known as user profil fields)', VALUE_OPTIONAL), 'groups' => new external_multiple_structure( new external_single_structure( - array( + [ 'id' => new external_value(PARAM_INT, 'group id'), 'name' => new external_value(PARAM_RAW, 'group name'), 'description' => new external_value(PARAM_RAW, 'group description'), 'descriptionformat' => new external_format_value('description'), - ) + ] ), 'user groups', VALUE_OPTIONAL), 'roles' => new external_multiple_structure( new external_single_structure( - array( + [ 'roleid' => new external_value(PARAM_INT, 'role id'), 'name' => new external_value(PARAM_RAW, 'role name'), 'shortname' => new external_value(PARAM_ALPHANUMEXT, 'role shortname'), 'sortorder' => new external_value(PARAM_INT, 'role sortorder') - ) + ] ), 'user roles', VALUE_OPTIONAL), 'preferences' => new external_multiple_structure( new external_single_structure( - array( + [ 'name' => new external_value(PARAM_RAW, 'The name of the preferences'), 'value' => new external_value(PARAM_RAW, 'The value of the custom field'), - ) + ] ), 'User preferences', VALUE_OPTIONAL), 'enrolledcourses' => new external_multiple_structure( new external_single_structure( - array( + [ 'id' => new external_value(PARAM_INT, 'Id of the course'), 'fullname' => new external_value(PARAM_RAW, 'Fullname of the course'), 'shortname' => new external_value(PARAM_RAW, 'Shortname of the course') - ) + ] ), 'Courses where the user is enrolled - limited by which courses the user is able to see', VALUE_OPTIONAL) - ) + ] ) ); } diff --git a/enrol/tests/externallib_test.php b/enrol/tests/externallib_test.php index e80fb3dbdb4..c98df46ca47 100644 --- a/enrol/tests/externallib_test.php +++ b/enrol/tests/externallib_test.php @@ -366,12 +366,12 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase { // Create the course and the users. $course = $this->getDataGenerator()->create_course(); $coursecontext = context_course::instance($course->id); - $user0 = $this->getDataGenerator()->create_user(array('username' => 'user0active')); - $user1 = $this->getDataGenerator()->create_user(array('username' => 'user1active')); - $user2 = $this->getDataGenerator()->create_user(array('username' => 'user2active')); - $user2su = $this->getDataGenerator()->create_user(array('username' => 'user2suspended')); // Suspended user. - $user3 = $this->getDataGenerator()->create_user(array('username' => 'user3active')); - $user3su = $this->getDataGenerator()->create_user(array('username' => 'user3suspended')); // Suspended user. + $user0 = $this->getDataGenerator()->create_user(['username' => 'user0active']); + $user1 = $this->getDataGenerator()->create_user(['username' => 'user1active']); + $user2 = $this->getDataGenerator()->create_user(['username' => 'user2active']); + $user2su = $this->getDataGenerator()->create_user(['username' => 'user2suspended']); // Suspended user. + $user3 = $this->getDataGenerator()->create_user(['username' => 'user3active']); + $user3su = $this->getDataGenerator()->create_user(['username' => 'user3suspended']); // Suspended user. // Enrol the users in the course. $this->getDataGenerator()->enrol_user($user0->id, $course->id); @@ -392,10 +392,10 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase { role_assign($roleid, $USER->id, $coursecontext); // Suspended users. - $options = array( - array('name' => 'onlysuspended', 'value' => true), - array('name' => 'userfields', 'value' => 'id,username') - ); + $options = [ + ['name' => 'onlysuspended', 'value' => true], + ['name' => 'userfields', 'value' => 'id,username'] + ]; $suspendedusers = core_enrol_external::get_enrolled_users($course->id, $options); // We need to execute the return values cleaning process to simulate the web service server. @@ -407,10 +407,10 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase { } // Active users. - $options = array( - array('name' => 'onlyactive', 'value' => true), - array('name' => 'userfields', 'value' => 'id,username') - ); + $options = [ + ['name' => 'onlyactive', 'value' => true], + ['name' => 'userfields', 'value' => 'id,username'] + ]; $activeusers = core_enrol_external::get_enrolled_users($course->id, $options); // We need to execute the return values cleaning process to simulate the web service server. @@ -422,9 +422,9 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase { } // All enrolled users. - $options = array( - array('name' => 'userfields', 'value' => 'id,username') - ); + $options = [ + ['name' => 'userfields', 'value' => 'id,username'] + ]; $allusers = core_enrol_external::get_enrolled_users($course->id, $options); // We need to execute the return values cleaning process to simulate the web service server. From aa1691d8112c44aaf33d04c9da6ff51a4c07076b Mon Sep 17 00:00:00 2001 From: Ilya Tregubov Date: Fri, 27 Nov 2020 10:03:27 +0200 Subject: [PATCH 3/3] MDL-69869 enrol: Fixing intendation for travis. --- enrol/externallib.php | 86 +++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/enrol/externallib.php b/enrol/externallib.php index 99988ad099f..5189a3a7edd 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -745,50 +745,50 @@ class core_enrol_external extends external_api { $sortdirection = 'ASC'; foreach ($options as $option) { switch ($option['name']) { - case 'withcapability': - $withcapability = $option['value']; - break; - case 'groupid': - $groupid = (int)$option['value']; - break; - case 'onlyactive': - $onlyactive = !empty($option['value']); - break; - case 'onlysuspended': - $onlysuspended = !empty($option['value']); - break; - case 'userfields': - $thefields = explode(',', $option['value']); - foreach ($thefields as $f) { - $userfields[] = clean_param($f, PARAM_ALPHANUMEXT); - } - break; - case 'limitfrom' : - $limitfrom = clean_param($option['value'], PARAM_INT); - break; - case 'limitnumber' : - $limitnumber = clean_param($option['value'], PARAM_INT); - break; - case 'sortby': - $sortallowedvalues = ['id', 'firstname', 'lastname', 'siteorder']; - if (!in_array($option['value'], $sortallowedvalues)) { - throw new invalid_parameter_exception('Invalid value for sortby parameter (value: ' . $option['value'] . '),' . - 'allowed values are: ' . implode(',', $sortallowedvalues)); - } - if ($option['value'] == 'siteorder') { - list($sortby, $sortparams) = users_order_by_sql('us'); - } else { - $sortby = 'us.' . $option['value']; - } - break; - case 'sortdirection': - $sortdirection = strtoupper($option['value']); - $directionallowedvalues = ['ASC', 'DESC']; - if (!in_array($sortdirection, $directionallowedvalues)) { - throw new invalid_parameter_exception('Invalid value for sortdirection parameter + case 'withcapability': + $withcapability = $option['value']; + break; + case 'groupid': + $groupid = (int)$option['value']; + break; + case 'onlyactive': + $onlyactive = !empty($option['value']); + break; + case 'onlysuspended': + $onlysuspended = !empty($option['value']); + break; + case 'userfields': + $thefields = explode(',', $option['value']); + foreach ($thefields as $f) { + $userfields[] = clean_param($f, PARAM_ALPHANUMEXT); + } + break; + case 'limitfrom' : + $limitfrom = clean_param($option['value'], PARAM_INT); + break; + case 'limitnumber' : + $limitnumber = clean_param($option['value'], PARAM_INT); + break; + case 'sortby': + $sortallowedvalues = ['id', 'firstname', 'lastname', 'siteorder']; + if (!in_array($option['value'], $sortallowedvalues)) { + throw new invalid_parameter_exception('Invalid value for sortby parameter (value: ' . + $option['value'] . '), allowed values are: ' . implode(',', $sortallowedvalues)); + } + if ($option['value'] == 'siteorder') { + list($sortby, $sortparams) = users_order_by_sql('us'); + } else { + $sortby = 'us.' . $option['value']; + } + break; + case 'sortdirection': + $sortdirection = strtoupper($option['value']); + $directionallowedvalues = ['ASC', 'DESC']; + if (!in_array($sortdirection, $directionallowedvalues)) { + throw new invalid_parameter_exception('Invalid value for sortdirection parameter (value: ' . $sortdirection . '),' . 'allowed values are: ' . implode(',', $directionallowedvalues)); - } - break; + } + break; } }