user_get_user_details doesn't return idnumber

Thanks for reporting this.

I've put that on the backlog.

In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

Thanks Crafton. I had a look to the code following this report and I noticed that only get_users_by_id() external function return the idnumber. The following functions need to return it too:

• get_enrolled_users
• get_course_user_profiles

Hi, sure i'll provide a patch.

Cool, I was going to do it but as you nicely volunteer I let it to you Crafton. Please submit the patch on github. I am reverting this issue status to not in development

Will do.

Here's the patch. https://github.com/crafton/moodle/commit/f9d425070643bda137281be71153b7c417fb6f14.

Spent most of the time setting up git.

I think though in user_get_user_details, instead of an admin & currentuser check for Department, Institution and Idnumber, I suggest we go a moodle/site:viewuseridentity capability check. I think this is a little cleaner and would allow the admin to manage this from the admin interface.

Thanks Crafton for the 'idnumber' report, the fix and the suggestion about the missing capability. I added the missing capability checks.

no problem, glad i could assist.

Hi Jerome,

Running the following commands produced these errors:

phpunit core_user_external_testcase user/tests/externallib_test.php
Cannot open file "user/tests/externallib_test.php".

phpunit core_enrol_external_testcase enrol/tests/enrolexternallib_test.php
Cannot open file "user/tests/externallib_test.php".

Please ignore the previous comment. It needs MDL-33529 to run this issue properly.

The patch looks good.

When will this be in a new weekly build release?
Is get_enrolled_users also solved and returning idnumber?

Sending to integration. Thanks for the reminder Nuku.

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

Thanks Crafton and Jerome, i've integrated this now.

(It'd be great to have the phpunit branch integrated for testing this )

Hi,
This works as expected.
@Dan,
can you please pass this test?
Thanks

Done.

asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ!

Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

Hello,

it seems that its not working anymore on 2.4.
At user/lib.php , line 240 we have

$showuseridentityfields = get_extra_user_fields($context);

That is an array of fields, but later at line 413 the condition tests if the field is a key

if ($isadmin or $currentuser or isset($showuseridentityfields['idnumber']))

when it should be

if ($isadmin or $currentuser or in_array('idnumber',$showuseridentityfields))

This is true for other fields like institution, department, phone1 and phone2.

Hello,

it seems that its not working anymore on 2.4.
At user/lib.php , line 240 we have

$showuseridentityfields = get_extra_user_fields($context);

That is an array of fields, but at line 240 the condition tests if the field is a key

if ($isadmin or $currentuser or isset($showuseridentityfields['idnumber']))

when it should test if the value exist in array

if ($isadmin or $currentuser or in_array('idnumber', $showuseridentityfields))

Same thing happens for institution, department, phone1 and phone2

How did this ever pass testing???

Thanks Daniel, please could you file a new bug for this? Seems like we should have some unit tests for this to.