core_user_get_users_by_field()

Hi Jerome,

Some feedback for the patch.
user/externallib.php

1. Line 274: @return should be stated last and it is missing return type
2. Line 375: @since should be 2.4?
3. Line 381: 'the search field can be:id, idnumber, username, email' -> it needs 'or' indication
4. Line 394: @since should be stated right after the function definition.
5. Line 457: missing @return type
6. Line 458: @since should be stated right after the function definition with 2.4 version?
7. Line 469: email - can it use PARAM_EMAIL instead of PARAM_TEXT?
8. Line 491: descriptionformat - missing param type on first param

user/lib.php

1. Line 489: typo for 'through'
2. Line 491: param type definition for documentation
3. Line 492: return type should be null|array
4. Line 502 - 513: make the logic friendlier
5. line 525: Change the definition for the function.
6. Line 526: remove the question
7. Line 528: missing param type for $user
8. Line 529: $course param type should be null|stdClass
9. Line 530: return definition should be word out instead of using T for true

Other than that it looks great.

Check list:
[y] Syntax
[y] Output
[y] Whitespace
[-] Language
[-] Databases
[y] Testing
[-] Security
[-] Documentation
[y] Git
[y] Sanity check

Thanks Rosie. I didn't check the codechecker/moodlechecker on it, I'm too used to get the smurf file from the integrators. Below, some explanation:

user/externallib.php:
1. http://docs.moodle.org/dev/Coding_style#.40return does not mention that @return has to be the last annotation. The return type external_function_parameters is correct. You can write the class name instead of 'object', it's actually better in my opinion.
2. We promised the web service contributors that we'll backport web service function to 2.3 when possible, so I mention 2.3 yet (I don't know yet if it will pass the integration). I'll change for the right version => 2.3.X.
3. Fixing. As the 'field' parameter is a string and is not an occurrence (it's not a list) I thought it was implicit enough, but I'll add an 'or'
4. http://docs.moodle.org/dev/Coding_style#.40since does not mention anything about placement too.
5. same as 1.
6. same as 2.
7. As it is mentioned in the description of this field "An email address - allow email as root@localhost". PARAM_EMAIL would fail root@localhost email address during the web service return value validation process. To be honest I'm not even sure about the rightness to do any email validation... We would need to check other email field of other external function and comment about that in Moodledocs.
8. The code is correct. Note that is a external_format_value() not a external_value()

user/lib.php:
1. Fixing.
2. Fixing.
3. Fixing.
4. Thanks for noticing the perf issue Rosie. I myself don't use this kind of logic, at the end we learned about the difference about |= and ||= (the first one test all condition, the second one stop at the first true), it was a good exercice
5. Fixing.
6. Fixing.
7. Fixing.
8. Fixing.

After talking on the dev chat, you were right Rosie, we should enforce PARAM_EMAIL everywhere, Moodle doesn't support email different from YYY @ DOMAIN.NAME

I only added this function to 2.4 as it does some change into user/lib.php. I'd like integrator opinion on this as it is pretty similar to MDL-29938 (core_user_get_users()).

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

$can_view_user_details_cap($user, $course)

will look at this properly tomorrow now, Jerome is this needed (absolutely essential) in 2.4.1, or can is master only ok?
Noting as this is a new feature it would land to just master.

Many thanks
Sam

Hi Sam,
it's not needed in 2.4.1.

Awesome thanks Jerome, adding the integration_held label now.

Ok, this has now landed. Thanks Jerome.

Thanks Jerome,

Phpunit passes and webservice return matching user records.

And your fantastic code has met core, hope they become good friends for a long period.

Closing, thanks!

The function has been added to ws docs: http://docs.moodle.org/dev/Web_services_Roadmap#Core_web_service_functions