MDL-77468 user: Make user profile visibility consistent web and ws
This commit is contained in:
+33
-21
@@ -400,21 +400,21 @@ function user_get_user_details($user, $course = null, array $userfields = array(
|
||||
// Hidden user field.
|
||||
if ($canviewhiddenuserfields) {
|
||||
$hiddenfields = array();
|
||||
// Address, phone1 and phone2 not appears in hidden fields list but require viewhiddenfields capability
|
||||
// according to user/profile.php.
|
||||
if (!empty($user->address) && in_array('address', $userfields)) {
|
||||
$userdetails['address'] = $user->address;
|
||||
}
|
||||
} else {
|
||||
$hiddenfields = array_flip(explode(',', $CFG->hiddenuserfields));
|
||||
}
|
||||
|
||||
if (!empty($user->phone1) && in_array('phone1', $userfields) &&
|
||||
(in_array('phone1', $showuseridentityfields) or $canviewhiddenuserfields)) {
|
||||
|
||||
if (!empty($user->address) && (in_array('address', $userfields)
|
||||
&& in_array('address', $showuseridentityfields) || $isadmin)) {
|
||||
$userdetails['address'] = $user->address;
|
||||
}
|
||||
if (!empty($user->phone1) && (in_array('phone1', $userfields)
|
||||
&& in_array('phone1', $showuseridentityfields) || $isadmin)) {
|
||||
$userdetails['phone1'] = $user->phone1;
|
||||
}
|
||||
if (!empty($user->phone2) && in_array('phone2', $userfields) &&
|
||||
(in_array('phone2', $showuseridentityfields) or $canviewhiddenuserfields)) {
|
||||
if (!empty($user->phone2) && (in_array('phone2', $userfields)
|
||||
&& in_array('phone2', $showuseridentityfields) || $isadmin)) {
|
||||
$userdetails['phone2'] = $user->phone2;
|
||||
}
|
||||
|
||||
@@ -436,6 +436,10 @@ function user_get_user_details($user, $course = null, array $userfields = array(
|
||||
$userdetails['city'] = $user->city;
|
||||
}
|
||||
|
||||
if (in_array('timezone', $userfields) && (!isset($hiddenfields['timezone']) || $isadmin) && $user->timezone) {
|
||||
$userdetails['timezone'] = $user->timezone;
|
||||
}
|
||||
|
||||
if (in_array('suspended', $userfields) && (!isset($hiddenfields['suspended']) or $isadmin)) {
|
||||
$userdetails['suspended'] = (bool)$user->suspended;
|
||||
}
|
||||
@@ -518,17 +522,25 @@ function user_get_user_details($user, $course = null, array $userfields = array(
|
||||
}
|
||||
}
|
||||
|
||||
// If groups are in use and enforced throughout the course, then make sure we can meet in at least one course level group.
|
||||
if (in_array('groups', $userfields) && !empty($course) && $canaccessallgroups) {
|
||||
$usergroups = groups_get_all_groups($course->id, $user->id, $course->defaultgroupingid,
|
||||
'g.id, g.name,g.description,g.descriptionformat');
|
||||
$userdetails['groups'] = array();
|
||||
foreach ($usergroups as $group) {
|
||||
list($group->description, $group->descriptionformat) =
|
||||
external_format_text($group->description, $group->descriptionformat,
|
||||
$context->id, 'group', 'description', $group->id);
|
||||
$userdetails['groups'][] = array('id' => $group->id, 'name' => $group->name,
|
||||
'description' => $group->description, 'descriptionformat' => $group->descriptionformat);
|
||||
// Return user groups.
|
||||
if (in_array('groups', $userfields) && !empty($course)) {
|
||||
if ($usergroups = groups_get_all_groups($course->id, $user->id)) {
|
||||
$userdetails['groups'] = [];
|
||||
foreach ($usergroups as $group) {
|
||||
if ($course->groupmode == SEPARATEGROUPS && !$canaccessallgroups && $user->id != $USER->id) {
|
||||
// In separate groups, I only have to see the groups shared between both users.
|
||||
if (!groups_is_member($group->id, $USER->id)) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
$userdetails['groups'][] = [
|
||||
'id' => $group->id,
|
||||
'name' => format_string($group->name),
|
||||
'description' => format_text($group->description, $group->descriptionformat, ['context' => $context]),
|
||||
'descriptionformat' => $group->descriptionformat
|
||||
];
|
||||
}
|
||||
}
|
||||
}
|
||||
// List of courses where the user is enrolled.
|
||||
@@ -560,7 +572,7 @@ function user_get_user_details($user, $course = null, array $userfields = array(
|
||||
}
|
||||
|
||||
if ($currentuser or has_capability('moodle/user:viewalldetails', $context)) {
|
||||
$extrafields = ['auth', 'confirmed', 'lang', 'theme', 'timezone', 'mailformat'];
|
||||
$extrafields = ['auth', 'confirmed', 'lang', 'theme', 'mailformat'];
|
||||
foreach ($extrafields as $extrafield) {
|
||||
if (in_array($extrafield, $userfields) && isset($user->$extrafield)) {
|
||||
$userdetails[$extrafield] = $user->$extrafield;
|
||||
|
||||
@@ -356,6 +356,12 @@ class externallib_test extends externallib_advanced_testcase {
|
||||
$this->getDataGenerator()->enrol_user($return->user2->id, $return->course->id, $return->roleid, 'manual');
|
||||
$this->getDataGenerator()->enrol_user($USER->id, $return->course->id, $return->roleid, 'manual');
|
||||
|
||||
$group1 = $this->getDataGenerator()->create_group(['courseid' => $return->course->id, 'name' => 'G1']);
|
||||
$group2 = $this->getDataGenerator()->create_group(['courseid' => $return->course->id, 'name' => 'G2']);
|
||||
|
||||
groups_add_member($group1->id, $return->user1->id);
|
||||
groups_add_member($group2->id, $return->user2->id);
|
||||
|
||||
return $return;
|
||||
}
|
||||
|
||||
@@ -399,6 +405,9 @@ class externallib_test extends externallib_advanced_testcase {
|
||||
|
||||
// We need to execute the return values cleaning process to simulate the web service server.
|
||||
$enrolledusers = \external_api::clean_returnvalue(core_user_external::get_course_user_profiles_returns(), $enrolledusers);
|
||||
// Check we get the requested user and that is in a group.
|
||||
$this->assertCount(1, $enrolledusers);
|
||||
$this->assertCount(1, $enrolledusers[0]['groups']);
|
||||
|
||||
foreach($enrolledusers as $enrolleduser) {
|
||||
if ($enrolleduser['username'] == $data->user1->username) {
|
||||
|
||||
@@ -869,4 +869,148 @@ class userlib_test extends \advanced_testcase {
|
||||
self::assertSame('5', $got['timezone']);
|
||||
self::assertSame('0', $got['mailformat']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test user_get_user_details_permissions
|
||||
* @covers ::user_get_user_details
|
||||
*/
|
||||
public function test_user_get_user_details_permissions() {
|
||||
global $CFG, $DB;
|
||||
|
||||
$this->resetAfterTest();
|
||||
|
||||
// Create user and modify user profile.
|
||||
$teacher = $this->getDataGenerator()->create_user();
|
||||
$student1 = $this->getDataGenerator()->create_user(['idnumber' => 'user1id', 'city' => 'Barcelona', 'address' => 'BCN 1B']);
|
||||
$student2 = $this->getDataGenerator()->create_user();
|
||||
$student1fullname = fullname($student1);
|
||||
|
||||
$course = $this->getDataGenerator()->create_course();
|
||||
$coursecontext = \context_course::instance($course->id);
|
||||
$this->getDataGenerator()->enrol_user($teacher->id, $course->id);
|
||||
$this->getDataGenerator()->enrol_user($student1->id, $course->id);
|
||||
$this->getDataGenerator()->enrol_user($student2->id, $course->id);
|
||||
$teacherrole = $DB->get_record('role', array('shortname' => 'teacher'));
|
||||
$studentrole = $DB->get_record('role', array('shortname' => 'student'));
|
||||
$this->getDataGenerator()->role_assign($teacherrole->id, $teacher->id, $coursecontext->id);
|
||||
$this->getDataGenerator()->role_assign($studentrole->id, $student1->id, $coursecontext->id);
|
||||
$this->getDataGenerator()->role_assign($studentrole->id, $student2->id, $coursecontext->id);
|
||||
|
||||
accesslib_clear_all_caches_for_unit_testing();
|
||||
|
||||
// Get student details as a user with super system capabilities.
|
||||
$result = user_get_user_details($student1, $course);
|
||||
$this->assertEquals($student1->id, $result['id']);
|
||||
$this->assertEquals($student1fullname, $result['fullname']);
|
||||
$this->assertEquals($course->id, $result['enrolledcourses'][0]['id']);
|
||||
|
||||
$this->setUser($student2);
|
||||
|
||||
// Get student details with required fields.
|
||||
$result = user_get_user_details($student1, $course, array('id', 'fullname', 'timezone', 'city', 'address', 'idnumber'));
|
||||
$this->assertCount(4, $result); // Ensure address (never returned), idnumber (identity field) are not returned here.
|
||||
$this->assertEquals($student1->id, $result['id']);
|
||||
$this->assertEquals($student1fullname, $result['fullname']);
|
||||
$this->assertEquals($student1->timezone, $result['timezone']);
|
||||
$this->assertEquals($student1->city, $result['city']);
|
||||
|
||||
// Set new identity fields and hidden fields and try to retrieve them without permission.
|
||||
$CFG->showuseridentity = $CFG->showuseridentity . ',idnumber';
|
||||
$CFG->hiddenuserfields = 'city';
|
||||
$result = user_get_user_details($student1, $course, array('id', 'fullname', 'timezone', 'city', 'address', 'idnumber'));
|
||||
$this->assertCount(3, $result); // Ensure address, city and idnumber are not returned here.
|
||||
$this->assertEquals($student1->id, $result['id']);
|
||||
$this->assertEquals($student1fullname, $result['fullname']);
|
||||
$this->assertEquals($student1->timezone, $result['timezone']);
|
||||
|
||||
// Now, teacher should have permission to see the idnumber and city fields.
|
||||
$this->setUser($teacher);
|
||||
$result = user_get_user_details($student1, $course, array('id', 'fullname', 'timezone', 'city', 'address', 'idnumber'));
|
||||
$this->assertCount(5, $result); // Ensure address is not returned here.
|
||||
$this->assertEquals($student1->id, $result['id']);
|
||||
$this->assertEquals($student1fullname, $result['fullname']);
|
||||
$this->assertEquals($student1->timezone, $result['timezone']);
|
||||
$this->assertEquals($student1->idnumber, $result['idnumber']);
|
||||
$this->assertEquals($student1->city, $result['city']);
|
||||
|
||||
// And admins can see anything.
|
||||
$this->setAdminUser();
|
||||
$result = user_get_user_details($student1, $course, array('id', 'fullname', 'timezone', 'city', 'address', 'idnumber'));
|
||||
$this->assertCount(6, $result);
|
||||
$this->assertEquals($student1->id, $result['id']);
|
||||
$this->assertEquals($student1fullname, $result['fullname']);
|
||||
$this->assertEquals($student1->timezone, $result['timezone']);
|
||||
$this->assertEquals($student1->idnumber, $result['idnumber']);
|
||||
$this->assertEquals($student1->city, $result['city']);
|
||||
$this->assertEquals($student1->address, $result['address']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test user_get_user_details_groups.
|
||||
* @covers ::user_get_user_details
|
||||
*/
|
||||
public function test_user_get_user_details_groups() {
|
||||
global $DB;
|
||||
$this->resetAfterTest();
|
||||
|
||||
// Create user and modify user profile.
|
||||
$teacher = $this->getDataGenerator()->create_user();
|
||||
$student1 = $this->getDataGenerator()->create_user(['idnumber' => 'user1id', 'city' => 'Barcelona', 'address' => 'BCN 1B']);
|
||||
$student2 = $this->getDataGenerator()->create_user();
|
||||
|
||||
$course = $this->getDataGenerator()->create_course();
|
||||
$coursecontext = \context_course::instance($course->id);
|
||||
$this->getDataGenerator()->enrol_user($teacher->id, $course->id);
|
||||
$this->getDataGenerator()->enrol_user($student1->id, $course->id);
|
||||
$this->getDataGenerator()->enrol_user($student2->id, $course->id);
|
||||
$teacherrole = $DB->get_record('role', array('shortname' => 'teacher'));
|
||||
$studentrole = $DB->get_record('role', array('shortname' => 'student'));
|
||||
$this->getDataGenerator()->role_assign($teacherrole->id, $teacher->id, $coursecontext->id);
|
||||
$this->getDataGenerator()->role_assign($studentrole->id, $student1->id, $coursecontext->id);
|
||||
$this->getDataGenerator()->role_assign($studentrole->id, $student2->id, $coursecontext->id);
|
||||
|
||||
$group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id, 'name' => 'G1']);
|
||||
$group2 = $this->getDataGenerator()->create_group(['courseid' => $course->id, 'name' => 'G2']);
|
||||
|
||||
// Each student in one group but teacher in two.
|
||||
groups_add_member($group1->id, $student1->id);
|
||||
groups_add_member($group1->id, $teacher->id);
|
||||
groups_add_member($group2->id, $student2->id);
|
||||
groups_add_member($group2->id, $teacher->id);
|
||||
|
||||
accesslib_clear_all_caches_for_unit_testing();
|
||||
|
||||
// A student can see other users groups when separate groups are not forced.
|
||||
$this->setUser($student2);
|
||||
|
||||
// Get student details with groups.
|
||||
$result = user_get_user_details($student1, $course, array('id', 'fullname', 'groups'));
|
||||
$this->assertCount(3, $result);
|
||||
$this->assertEquals($group1->id, $result['groups'][0]['id']);
|
||||
|
||||
// Teacher is in two different groups.
|
||||
$result = user_get_user_details($teacher, $course, array('id', 'fullname', 'groups'));
|
||||
|
||||
// Order by group id.
|
||||
usort($result['groups'], function($a, $b) {
|
||||
return $a['id'] - $b['id'];
|
||||
});
|
||||
|
||||
$this->assertCount(3, $result);
|
||||
$this->assertCount(2, $result['groups']);
|
||||
$this->assertEquals($group1->id, $result['groups'][0]['id']);
|
||||
$this->assertEquals($group2->id, $result['groups'][1]['id']);
|
||||
|
||||
// Change to separate groups.
|
||||
$course->groupmode = SEPARATEGROUPS;
|
||||
$course->groupmodeforce = true;
|
||||
update_course($course);
|
||||
|
||||
// Teacher is in two groups but I can only see the one shared with me.
|
||||
$result = user_get_user_details($teacher, $course, array('id', 'fullname', 'groups'));
|
||||
|
||||
$this->assertCount(3, $result);
|
||||
$this->assertCount(1, $result['groups']);
|
||||
$this->assertEquals($group2->id, $result['groups'][0]['id']);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user