Skip to content

Commit

Permalink
MDL-29938 throw exception when a criteria is repeated, used self inst…
Browse files Browse the repository at this point in the history
…ead classname when possible, return only not deleted users
  • Loading branch information
mouneyrac committed Feb 15, 2013
1 parent 0c34e80 commit 85e6bf8
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 14 deletions.
35 changes: 21 additions & 14 deletions user/externallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ public static function get_users_by_field($field, $values) {
* @since Moodle 2.4
*/
public static function get_users_by_field_returns() {
return new external_multiple_structure(core_user_external::user_description());
return new external_multiple_structure(self::user_description());
}


Expand Down Expand Up @@ -485,9 +485,11 @@ public static function get_users_parameters() {
)
), 'the key/value pairs to be considered in user search. Values can not be empty.
Specify different keys only once (fullname => \'user1\', auth => \'manual\', ...) -
key occurences are ignored, only the last occurence is considered.
key occurences are forbidden.
The search is executed with AND operator on the criterias. Invalid criterias (keys) are ignored,
the search is still executed on the valid criterias.'
the search is still executed on the valid criterias.
You can search without criteria, but the function is not designed for it.
It could very slow or timeout. The function is designed to search some specific users.'
)
)
);
Expand All @@ -509,13 +511,23 @@ public static function get_users($criteria = array()) {
array('criteria' => $criteria));

// Validate the criteria and retrieve the users.
$firstcriteria = true;
$users = array();
$warnings = array();
$sql = '';
$sqlparams = array();
$usedkeys = array();

// Do not retrieve deleted users.
$sql = ' deleted = 0';

foreach ($params['criteria'] as $criteriaindex => $criteria) {

// Check that the criteria has never been used.
if (array_key_exists($criteria['key'], $usedkeys)) {
throw new moodle_exception('keyalreadyset', '', '', null, 'The key ' . $criteria['key'] . ' can only be sent once');
} else {
$usedkeys[$criteria['key']] = true;
}

$invalidcriteria = false;
// Clean the parameters.
$paramtype = PARAM_RAW;
Expand Down Expand Up @@ -557,12 +569,7 @@ public static function get_users($criteria = array()) {
if (!$invalidcriteria) {
$cleanedvalue = clean_param($criteria['value'], $paramtype);

// If first criteria do not add AND to the query.
if ($firstcriteria) {
$firstcriteria = false;
} else {
$sql .= ' AND ';
}
$sql .= ' AND ';

// Create the SQL.
switch ($criteria['key']) {
Expand Down Expand Up @@ -621,7 +628,7 @@ public static function get_users($criteria = array()) {
public static function get_users_returns() {
return new external_single_structure(
array('users' => new external_multiple_structure(
core_user_external::user_description()
self::user_description()
),
'warnings' => new external_warnings('always set to \'key\'', 'faulty key name')
)
Expand Down Expand Up @@ -712,7 +719,7 @@ public static function get_users_by_id_returns() {
'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));
return new external_multiple_structure(self::user_description($additionalfields));
}

/**
Expand Down Expand Up @@ -832,7 +839,7 @@ public static function get_course_user_profiles_returns() {
), '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));
return new external_multiple_structure(self::user_description($additionalfields));
}

/**
Expand Down
16 changes: 16 additions & 0 deletions user/tests/externallib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,22 @@ public function test_get_users() {
$warning = array_pop($warnings);
$this->assertEquals($warning['item'], 'invalidkey');
$this->assertEquals($warning['warningcode'], 'invalidfieldparameter');

// Test sending twice the same search field.
try {
$searchparams = array(
array('key' => 'firstname', 'value' => 'Canard'),
array('key' => 'email', 'value' => $user1->email),
array('key' => 'firstname', 'value' => $user1->firstname));

// Call the external function.
$result = core_user_external::get_users($searchparams);
$this->fail('Expecting \'keyalreadyset\' moodle_exception to be thrown.');
} catch (moodle_exception $e) {
$this->assertEquals('keyalreadyset', $e->errorcode);
} catch (Exception $e) {
$this->fail('Expecting \'keyalreadyset\' moodle_exception to be thrown.');
}
}

/**
Expand Down

0 comments on commit 85e6bf8

Please sign in to comment.