Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

MDL-29938 Fix typo bypassing some code + remove misplaced code in dep…

…recated function + some code cleaning
  • Loading branch information...
commit bb1105ae31e317e818e6a3609a07bc8e574749d6 1 parent ec80bc6
@mouneyrac mouneyrac authored
Showing with 54 additions and 75 deletions.
  1. +44 −52 user/externallib.php
  2. +10 −23 user/tests/externallib_test.php
View
96 user/externallib.php
@@ -438,7 +438,6 @@ public static function get_users_by_field($field, $values) {
// Finally retrieve each users information
$returnedusers = array();
foreach ($users as $user) {
-
$userdetails = user_get_user_details_courses($user);
// Return the user only if the searched field is returned
@@ -463,10 +462,10 @@ public static function get_users_by_field_returns() {
/**
- * Returns description of get_users() parameters
+ * Returns description of get_users() parameters.
*
* @return external_function_parameters
- * @since Moodle 2.4
+ * @since Moodle 2.5
*/
public static function get_users_parameters() {
return new external_function_parameters(
@@ -474,18 +473,18 @@ public static function get_users_parameters() {
'criteria' => new external_multiple_structure(
new external_single_structure(
array(
- 'key' => new external_value(PARAM_ALPHA, 'the user column to search, expected keys (value format) are:
- "id" (int) matching user id,
- "lastname" (string) user last name (Note: you can use % for searching but it can be slow!),
- "firstname" (string) user first name (Note: you can use % for searching but it can be slow!),
- "idnumber" (string) matching user idnumber,
- "username" (string) matching user username,
- "email" (string) user email (Note: you can use % for searching but it can be slow!),
- "auth" (plugin) matching user auth plugin'),
- 'value' => new external_value(PARAM_RAW, 'the value to search')
+ 'key' => new external_value(PARAM_ALPHA, 'the user column to search, expected keys (value format) are:
+ "id" (int) matching user id,
+ "lastname" (string) user last name (Note: you can use % for searching but it may be considerably slower!),
+ "firstname" (string) user first name (Note: you can use % for searching but it may be considerably slower!),
+ "idnumber" (string) matching user idnumber,
+ "username" (string) matching user username,
+ "email" (string) user email (Note: you can use % for searching but it may be considerably slower!),
+ "auth" (string) matching user auth plugin'),
+ 'value' => new external_value(PARAM_RAW, 'the value to search')
)
), 'the key/value pairs to be considered in user search. Values can not be empty.
- Specifiy different keys only once (fullname => \'user1\', auth => \'manual\', ...) -
+ Specify different keys only once (fullname => \'user1\', auth => \'manual\', ...) -
key occurences are ignored, only the last occurence is considered.
The search is executed with AND operator on the criterias.'
)
@@ -494,12 +493,11 @@ public static function get_users_parameters() {
}
/**
- * Retrieve matching user
+ * Retrieve matching user.
*
- * @param string $field
- * @param array $values
- * @return array An array of arrays containg user profiles.
- * @since Moodle 2.4
+ * @param array $criteria the allowed array keys are id/lastname/firstname/idnumber/username/email/auth.
+ * @return array An array of arrays containing user profiles.
+ * @since Moodle 2.5
*/
public static function get_users($criteria = array()) {
global $CFG, $USER, $DB;
@@ -509,8 +507,7 @@ public static function get_users($criteria = array()) {
$params = self::validate_parameters(self::get_users_parameters(),
array('criteria' => $criteria));
- // Validate the criteria and retrieve the users
- $cleanedvalues = array();
+ // Validate the criteria and retrieve the users.
$firstcriteria = true;
$users = array();
$warnings = array();
@@ -518,8 +515,7 @@ public static function get_users($criteria = array()) {
$sqlparams = array();
foreach ($params['criteria'] as $criteria) {
-
- // Clean the parameters
+ // Clean the parameters.
$paramtype = PARAM_RAW;
switch ($criteria['key']) {
case 'id':
@@ -532,7 +528,7 @@ public static function get_users($criteria = array()) {
$paramtype = PARAM_RAW;
break;
case 'email':
- // we use PARAM_RAW to allow searches with %
+ // We use PARAM_RAW to allow searches with %.
$paramtype = PARAM_RAW;
break;
case 'auth':
@@ -543,25 +539,25 @@ public static function get_users($criteria = array()) {
$paramtype = PARAM_TEXT;
break;
default:
- // Send back a warning that this search key is not supported in this version
- // This warning will make the function extandable without breaking clients
+ // Send back a warning that this search key is not supported in this version.
+ // This warning will make the function extandable without breaking clients.
$warnings[] = array(
'item' => 'key',
'itemid' => $criteria['key'],
'warningcode' => 'invalidfieldparameter',
- 'message' => 'The search key \'' . $$criteria['key'] . '\' is not supported, look at the web service documentation'
+ 'message' => 'The search key \'' . $criteria['key'] . '\' is not supported, look at the web service documentation'
);
}
$cleanedvalue = clean_param($criteria['value'], $paramtype);
- // If first criteria do not add AND to the query
+ // If first criteria do not add AND to the query.
if ($firstcriteria) {
$firstcriteria = false;
} else {
$sql .= ' AND ';
}
- // Create the SQL
+ // Create the SQL.
switch ($criteria['key']) {
case 'id':
case 'idnumber':
@@ -573,7 +569,7 @@ public static function get_users($criteria = array()) {
case 'email':
case 'lastname':
case 'firstname':
- $sql .= $DB->sql_like($criteria['key'], ':'.$criteria['key'], false);
+ $sql .= $DB->sql_like($criteria['key'], ':' . $criteria['key'], false);
$sqlparams[$criteria['key']] = $cleanedvalue;
break;
default:
@@ -583,10 +579,9 @@ public static function get_users($criteria = array()) {
$users = $DB->get_records_select('user', $sql, $sqlparams, 'id ASC');
- // Finally retrieve each users information
+ // Finally retrieve each users information.
$returnedusers = array();
foreach ($users as $user) {
-
$userdetails = user_get_user_details_courses($user);
// Return the user only if all the searched fields are returned.
@@ -610,17 +605,17 @@ public static function get_users($criteria = array()) {
}
/**
- * Returns description of get_users result value
+ * Returns description of get_users result value.
*
* @return external_description
- * @since Moodle 2.3
+ * @since Moodle 2.5
*/
public static function get_users_returns() {
return new external_single_structure(
array('users' => new external_multiple_structure(
core_user_external::user_description()
),
- 'warnings' => new external_warnings()
+ 'warnings' => new external_warnings('always set to \'key\'', 'faulty key name')
)
);
}
@@ -693,8 +688,6 @@ public static function get_users_by_id($userids) {
return $result;
}
-
-
/**
* Returns description of method result value
*
@@ -702,10 +695,18 @@ public static function get_users_by_id($userids) {
* @since Moodle 2.2
*/
public static function get_users_by_id_returns() {
- return new external_multiple_structure(
- core_user_external::user_description()
- );
+ $additionalfields = array (
+ '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));
+ return new external_multiple_structure(core_user_external::user_description($additionalfields));
}
+
/**
* Returns description of method parameters
*
@@ -829,10 +830,10 @@ public static function get_course_user_profiles_returns() {
/**
* Create user return value description.
*
- * @param array $additionalfiels some additional field
+ * @param array $additionalfields some additional field
* @return single_structure_description
*/
- public static function user_description($additionalfiels = array()) {
+ public static function user_description($additionalfields = array()) {
$userfields = array(
'id' => new external_value(PARAM_INT, 'ID of the user'),
'username' => new external_value(PARAM_RAW, 'The username', VALUE_OPTIONAL),
@@ -875,7 +876,7 @@ public static function user_description($additionalfiels = array()) {
'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),
+ ), 'User custom fields (also known as user profile fields)', VALUE_OPTIONAL),
'preferences' => new external_multiple_structure(
new external_single_structure(
array(
@@ -1067,16 +1068,7 @@ public static function get_users_by_id($userids) {
* @see core_user_external::get_users_by_id_returns()
*/
public static function get_users_by_id_returns() {
- $additionalfields = array (
- '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));
- return core_user_external::get_users_by_id_returns($additionalfields);
+ return core_user_external::get_users_by_id_returns();
}
/**
* Returns description of method parameters
View
33 user/tests/externallib_test.php
@@ -42,6 +42,7 @@ public function test_get_users() {
$this->resetAfterTest(true);
$course = self::getDataGenerator()->create_course();
+
$user1 = array(
'username' => 'usernametest1',
'idnumber' => 'idnumbertest1',
@@ -64,6 +65,7 @@ public function test_get_users() {
'url' => 'http://moodle.org',
'country' => 'au'
);
+
$user1 = self::getDataGenerator()->create_user($user1);
if (!empty($CFG->usetags)) {
require_once($CFG->dirroot . '/user/editlib.php');
@@ -71,6 +73,7 @@ public function test_get_users() {
$user1->interests = array('Cinema', 'Tennis', 'Dance', 'Guitar', 'Cooking');
useredit_update_interests($user1, $user1->interests);
}
+
$user2 = self::getDataGenerator()->create_user(
array('username' => 'usernametest2', 'idnumber' => 'idnumbertest2'));
@@ -82,18 +85,9 @@ public function test_get_users() {
$roleid = $this->assignUserCapability('moodle/user:viewdetails', $context->id);
// Enrol the users in the course.
- // We use the manual plugin.
- $enrol = enrol_get_plugin('manual');
- $enrolinstances = enrol_get_instances($course->id, true);
- foreach ($enrolinstances as $courseenrolinstance) {
- if ($courseenrolinstance->enrol == "manual") {
- $instance = $courseenrolinstance;
- break;
- }
- }
- $enrol->enrol_user($instance, $user1->id, $roleid);
- $enrol->enrol_user($instance, $user2->id, $roleid);
- $enrol->enrol_user($instance, $USER->id, $roleid);
+ $this->getDataGenerator()->enrol_user($user1->id, $course->id, $roleid);
+ $this->getDataGenerator()->enrol_user($user2->id, $course->id, $roleid);
+ $this->getDataGenerator()->enrol_user($USER->id, $course->id, $roleid);
// call as admin and receive all possible fields.
$this->setAdminUser();
@@ -105,6 +99,9 @@ public function test_get_users() {
// Call the external function.
$result = core_user_external::get_users($searchparams);
+ // We need to execute the return values cleaning process to simulate the web service server
+ $result = external_api::clean_returnvalue(core_user_external::get_users_returns(), $result);
+
// Check we retrieve the good total number of enrolled users + no error on capability.
$expectedreturnedusers = 1;
$returnedusers = $result['users'];
@@ -119,7 +116,7 @@ public function test_get_users() {
}
$this->assertEquals($generateduser->firstname, $returneduser['firstname']);
$this->assertEquals($generateduser->lastname, $returneduser['lastname']);
- if ($generateduser->email != $USER->email) { //don't check the tmp modified $USER email
+ if ($generateduser->email != $USER->email) { // Don't check the tmp modified $USER email.
$this->assertEquals($generateduser->email, $returneduser['email']);
}
if (!empty($generateduser->address)) {
@@ -171,16 +168,6 @@ public function test_get_users() {
$this->assertEquals(implode(', ', $generateduser->interests), $returneduser['interests']);
}
}
-
- // Test that no result are returned for search by username if we are not admin
- $this->setGuestUser();
-
- // Call the external function.
- $returnedusers = core_user_external::get_users_by_field('username',
- array($USER->username, $user1->username, $user2->username));
-
- // Only the own $USER username should be returned
- $this->assertEquals(1, count($returnedusers));
}
/**
Please sign in to comment.
Something went wrong with that request. Please try again.