MDL-29938 Fix typo bypassing some code + remove misplaced code in deprecated function + some code cleaning

This commit is contained in:
Jerome Mouneyrac 2013-02-06 17:18:09 +08:00
parent ec80bc6bf8
commit bb1105ae31
2 changed files with 54 additions and 75 deletions

View file

@ -438,7 +438,6 @@ class core_user_external extends external_api {
// Finally retrieve each users information // Finally retrieve each users information
$returnedusers = array(); $returnedusers = array();
foreach ($users as $user) { foreach ($users as $user) {
$userdetails = user_get_user_details_courses($user); $userdetails = user_get_user_details_courses($user);
// Return the user only if the searched field is returned // Return the user only if the searched field is returned
@ -463,10 +462,10 @@ class core_user_external extends external_api {
/** /**
* Returns description of get_users() parameters * Returns description of get_users() parameters.
* *
* @return external_function_parameters * @return external_function_parameters
* @since Moodle 2.4 * @since Moodle 2.5
*/ */
public static function get_users_parameters() { public static function get_users_parameters() {
return new external_function_parameters( return new external_function_parameters(
@ -474,18 +473,18 @@ class core_user_external extends external_api {
'criteria' => new external_multiple_structure( 'criteria' => new external_multiple_structure(
new external_single_structure( new external_single_structure(
array( array(
'key' => new external_value(PARAM_ALPHA, 'the user column to search, expected keys (value format) are: 'key' => new external_value(PARAM_ALPHA, 'the user column to search, expected keys (value format) are:
"id" (int) matching user id, "id" (int) matching user id,
"lastname" (string) user last name (Note: you can use % for searching but it can be slow!), "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 can be slow!), "firstname" (string) user first name (Note: you can use % for searching but it may be considerably slower!),
"idnumber" (string) matching user idnumber, "idnumber" (string) matching user idnumber,
"username" (string) matching user username, "username" (string) matching user username,
"email" (string) user email (Note: you can use % for searching but it can be slow!), "email" (string) user email (Note: you can use % for searching but it may be considerably slower!),
"auth" (plugin) matching user auth plugin'), "auth" (string) matching user auth plugin'),
'value' => new external_value(PARAM_RAW, 'the value to search') '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. ), '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. key occurences are ignored, only the last occurence is considered.
The search is executed with AND operator on the criterias.' The search is executed with AND operator on the criterias.'
) )
@ -494,12 +493,11 @@ class core_user_external extends external_api {
} }
/** /**
* Retrieve matching user * Retrieve matching user.
* *
* @param string $field * @param array $criteria the allowed array keys are id/lastname/firstname/idnumber/username/email/auth.
* @param array $values * @return array An array of arrays containing user profiles.
* @return array An array of arrays containg user profiles. * @since Moodle 2.5
* @since Moodle 2.4
*/ */
public static function get_users($criteria = array()) { public static function get_users($criteria = array()) {
global $CFG, $USER, $DB; global $CFG, $USER, $DB;
@ -509,8 +507,7 @@ class core_user_external extends external_api {
$params = self::validate_parameters(self::get_users_parameters(), $params = self::validate_parameters(self::get_users_parameters(),
array('criteria' => $criteria)); array('criteria' => $criteria));
// Validate the criteria and retrieve the users // Validate the criteria and retrieve the users.
$cleanedvalues = array();
$firstcriteria = true; $firstcriteria = true;
$users = array(); $users = array();
$warnings = array(); $warnings = array();
@ -518,8 +515,7 @@ class core_user_external extends external_api {
$sqlparams = array(); $sqlparams = array();
foreach ($params['criteria'] as $criteria) { foreach ($params['criteria'] as $criteria) {
// Clean the parameters.
// Clean the parameters
$paramtype = PARAM_RAW; $paramtype = PARAM_RAW;
switch ($criteria['key']) { switch ($criteria['key']) {
case 'id': case 'id':
@ -532,7 +528,7 @@ class core_user_external extends external_api {
$paramtype = PARAM_RAW; $paramtype = PARAM_RAW;
break; break;
case 'email': case 'email':
// we use PARAM_RAW to allow searches with % // We use PARAM_RAW to allow searches with %.
$paramtype = PARAM_RAW; $paramtype = PARAM_RAW;
break; break;
case 'auth': case 'auth':
@ -543,25 +539,25 @@ class core_user_external extends external_api {
$paramtype = PARAM_TEXT; $paramtype = PARAM_TEXT;
break; break;
default: default:
// Send back a warning that this search key is not supported in this version // Send back a warning that this search key is not supported in this version.
// This warning will make the function extandable without breaking clients // This warning will make the function extandable without breaking clients.
$warnings[] = array( $warnings[] = array(
'item' => 'key', 'item' => 'key',
'itemid' => $criteria['key'], 'itemid' => $criteria['key'],
'warningcode' => 'invalidfieldparameter', '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); $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) { if ($firstcriteria) {
$firstcriteria = false; $firstcriteria = false;
} else { } else {
$sql .= ' AND '; $sql .= ' AND ';
} }
// Create the SQL // Create the SQL.
switch ($criteria['key']) { switch ($criteria['key']) {
case 'id': case 'id':
case 'idnumber': case 'idnumber':
@ -573,7 +569,7 @@ class core_user_external extends external_api {
case 'email': case 'email':
case 'lastname': case 'lastname':
case 'firstname': case 'firstname':
$sql .= $DB->sql_like($criteria['key'], ':'.$criteria['key'], false); $sql .= $DB->sql_like($criteria['key'], ':' . $criteria['key'], false);
$sqlparams[$criteria['key']] = $cleanedvalue; $sqlparams[$criteria['key']] = $cleanedvalue;
break; break;
default: default:
@ -583,10 +579,9 @@ class core_user_external extends external_api {
$users = $DB->get_records_select('user', $sql, $sqlparams, 'id ASC'); $users = $DB->get_records_select('user', $sql, $sqlparams, 'id ASC');
// Finally retrieve each users information // Finally retrieve each users information.
$returnedusers = array(); $returnedusers = array();
foreach ($users as $user) { foreach ($users as $user) {
$userdetails = user_get_user_details_courses($user); $userdetails = user_get_user_details_courses($user);
// Return the user only if all the searched fields are returned. // Return the user only if all the searched fields are returned.
@ -610,17 +605,17 @@ class core_user_external extends external_api {
} }
/** /**
* Returns description of get_users result value * Returns description of get_users result value.
* *
* @return external_description * @return external_description
* @since Moodle 2.3 * @since Moodle 2.5
*/ */
public static function get_users_returns() { public static function get_users_returns() {
return new external_single_structure( return new external_single_structure(
array('users' => new external_multiple_structure( array('users' => new external_multiple_structure(
core_user_external::user_description() core_user_external::user_description()
), ),
'warnings' => new external_warnings() 'warnings' => new external_warnings('always set to \'key\'', 'faulty key name')
) )
); );
} }
@ -693,8 +688,6 @@ class core_user_external extends external_api {
return $result; return $result;
} }
/** /**
* Returns description of method result value * Returns description of method result value
* *
@ -702,10 +695,18 @@ class core_user_external extends external_api {
* @since Moodle 2.2 * @since Moodle 2.2
*/ */
public static function get_users_by_id_returns() { public static function get_users_by_id_returns() {
return new external_multiple_structure( $additionalfields = array (
core_user_external::user_description() '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 * Returns description of method parameters
* *
@ -829,10 +830,10 @@ class core_user_external extends external_api {
/** /**
* Create user return value description. * Create user return value description.
* *
* @param array $additionalfiels some additional field * @param array $additionalfields some additional field
* @return single_structure_description * @return single_structure_description
*/ */
public static function user_description($additionalfiels = array()) { public static function user_description($additionalfields = array()) {
$userfields = array( $userfields = array(
'id' => new external_value(PARAM_INT, 'ID of the user'), 'id' => new external_value(PARAM_INT, 'ID of the user'),
'username' => new external_value(PARAM_RAW, 'The username', VALUE_OPTIONAL), 'username' => new external_value(PARAM_RAW, 'The username', VALUE_OPTIONAL),
@ -875,7 +876,7 @@ class core_user_external extends external_api {
'name' => new external_value(PARAM_RAW, 'The name 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'), '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( 'preferences' => new external_multiple_structure(
new external_single_structure( new external_single_structure(
array( array(
@ -1067,16 +1068,7 @@ class moodle_user_external extends external_api {
* @see core_user_external::get_users_by_id_returns() * @see core_user_external::get_users_by_id_returns()
*/ */
public static function get_users_by_id_returns() { public static function get_users_by_id_returns() {
$additionalfields = array ( return core_user_external::get_users_by_id_returns();
'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);
} }
/** /**
* Returns description of method parameters * Returns description of method parameters

View file

@ -42,6 +42,7 @@ class core_user_external_testcase extends externallib_advanced_testcase {
$this->resetAfterTest(true); $this->resetAfterTest(true);
$course = self::getDataGenerator()->create_course(); $course = self::getDataGenerator()->create_course();
$user1 = array( $user1 = array(
'username' => 'usernametest1', 'username' => 'usernametest1',
'idnumber' => 'idnumbertest1', 'idnumber' => 'idnumbertest1',
@ -64,6 +65,7 @@ class core_user_external_testcase extends externallib_advanced_testcase {
'url' => 'http://moodle.org', 'url' => 'http://moodle.org',
'country' => 'au' 'country' => 'au'
); );
$user1 = self::getDataGenerator()->create_user($user1); $user1 = self::getDataGenerator()->create_user($user1);
if (!empty($CFG->usetags)) { if (!empty($CFG->usetags)) {
require_once($CFG->dirroot . '/user/editlib.php'); require_once($CFG->dirroot . '/user/editlib.php');
@ -71,6 +73,7 @@ class core_user_external_testcase extends externallib_advanced_testcase {
$user1->interests = array('Cinema', 'Tennis', 'Dance', 'Guitar', 'Cooking'); $user1->interests = array('Cinema', 'Tennis', 'Dance', 'Guitar', 'Cooking');
useredit_update_interests($user1, $user1->interests); useredit_update_interests($user1, $user1->interests);
} }
$user2 = self::getDataGenerator()->create_user( $user2 = self::getDataGenerator()->create_user(
array('username' => 'usernametest2', 'idnumber' => 'idnumbertest2')); array('username' => 'usernametest2', 'idnumber' => 'idnumbertest2'));
@ -82,18 +85,9 @@ class core_user_external_testcase extends externallib_advanced_testcase {
$roleid = $this->assignUserCapability('moodle/user:viewdetails', $context->id); $roleid = $this->assignUserCapability('moodle/user:viewdetails', $context->id);
// Enrol the users in the course. // Enrol the users in the course.
// We use the manual plugin. $this->getDataGenerator()->enrol_user($user1->id, $course->id, $roleid);
$enrol = enrol_get_plugin('manual'); $this->getDataGenerator()->enrol_user($user2->id, $course->id, $roleid);
$enrolinstances = enrol_get_instances($course->id, true); $this->getDataGenerator()->enrol_user($USER->id, $course->id, $roleid);
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);
// call as admin and receive all possible fields. // call as admin and receive all possible fields.
$this->setAdminUser(); $this->setAdminUser();
@ -105,6 +99,9 @@ class core_user_external_testcase extends externallib_advanced_testcase {
// Call the external function. // Call the external function.
$result = core_user_external::get_users($searchparams); $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. // Check we retrieve the good total number of enrolled users + no error on capability.
$expectedreturnedusers = 1; $expectedreturnedusers = 1;
$returnedusers = $result['users']; $returnedusers = $result['users'];
@ -119,7 +116,7 @@ class core_user_external_testcase extends externallib_advanced_testcase {
} }
$this->assertEquals($generateduser->firstname, $returneduser['firstname']); $this->assertEquals($generateduser->firstname, $returneduser['firstname']);
$this->assertEquals($generateduser->lastname, $returneduser['lastname']); $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']); $this->assertEquals($generateduser->email, $returneduser['email']);
} }
if (!empty($generateduser->address)) { if (!empty($generateduser->address)) {
@ -171,16 +168,6 @@ class core_user_external_testcase extends externallib_advanced_testcase {
$this->assertEquals(implode(', ', $generateduser->interests), $returneduser['interests']); $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));
} }
/** /**