Skip to content

Commit

Permalink
Merge branch 'MDL-68333-master' of git://github.com/marinaglancy/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
@abgreeve
abgreeve committed May 20, 2020
2 parents 59d0090 + 5b529ac commit 1c1fd15
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 89 deletions.
4 changes: 2 additions & 2 deletions enrol/manual/tests/behat/quickenrolment.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,15 @@ Feature: Teacher can search and enrol users one by one into the course
When I log in as "admin"
Then the following "users" exist:
| username | firstname | lastname | email | phone1 | phone2 | department | institution | city | country |
| student100 | Student | 100 | student100@example.com | 1234567892 | 1234567893 | ABC1 | ABC2 | CITY1 | UK |
| student100 | Student | 100 | student100@example.com | 1234567892 | 1234567893 | ABC1 | ABC2 | CITY1 | GB |
And the following config values are set as admin:
| showuseridentity | idnumber,email,city,country,phone1,phone2,department,institution |
When I am on "Course 001" course homepage
Then I navigate to course participants
And I press "Enrol users"
When I set the field "Select users" to "[email protected]"
And I click on ".form-autocomplete-downarrow" "css_element" in the "Select users" "form_row"
Then I should see "[email protected], CITY1, UK, 1234567892, 1234567893, ABC1, ABC2"
Then I should see "[email protected], CITY1, GB, 1234567892, 1234567893, ABC1, ABC2"
# Remove identity field in setting User policies
And the following config values are set as admin:
| showuseridentity | idnumber,email,phone1,phone2,department,institution |
Expand Down
2 changes: 1 addition & 1 deletion grade/tests/report_graderlib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function test_process_data() {
$course = $this->getDataGenerator()->create_course();

// Create and enrol a student.
$student = $this->getDataGenerator()->create_user(array('username' => 'Student Sam'));
$student = $this->getDataGenerator()->create_user(array('username' => 'student_sam'));
$role = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST);
$this->getDataGenerator()->enrol_user($student->id, $course->id, $role->id);

Expand Down
4 changes: 2 additions & 2 deletions grade/tests/reportuserlib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ public function test_inject_rowspans() {
$coursecontext = context_course::instance($course->id);

// Create and enrol test users.
$student = $this->getDataGenerator()->create_user(array('username' => 'Student Sam'));
$student = $this->getDataGenerator()->create_user(array('username' => 'student_sam'));
$role = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST);
$this->getDataGenerator()->enrol_user($student->id, $course->id, $role->id);

$teacher = $this->getDataGenerator()->create_user(array('username' => 'Teacher T'));
$teacher = $this->getDataGenerator()->create_user(array('username' => 'teacher_t'));
$role = $DB->get_record('role', array('shortname' => 'editingteacher'), '*', MUST_EXIST);
$this->getDataGenerator()->enrol_user($teacher->id, $course->id, $role->id);

Expand Down
77 changes: 25 additions & 52 deletions lib/testing/generator/data_generator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public function get_plugin_generator($component) {
*/
public function create_user($record=null, array $options=null) {
global $DB, $CFG;
require_once($CFG->dirroot.'/user/lib.php');

$this->usercounter++;
$i = $this->usercounter;
Expand Down Expand Up @@ -206,10 +207,6 @@ public function create_user($record=null, array $options=null) {

if (isset($record['password'])) {
$record['password'] = hash_internal_user_password($record['password']);
} else {
// The auth plugin may not fully support this,
// but it is still better/faster than hashing random stuff.
$record['password'] = AUTH_PASSWORD_NOT_CACHED;
}

if (!isset($record['email'])) {
Expand All @@ -220,78 +217,54 @@ public function create_user($record=null, array $options=null) {
$record['confirmed'] = 1;
}

if (!isset($record['lang'])) {
$record['lang'] = 'en';
}

if (!isset($record['maildisplay'])) {
$record['maildisplay'] = $CFG->defaultpreference_maildisplay;
}

if (!isset($record['mailformat'])) {
$record['mailformat'] = $CFG->defaultpreference_mailformat;
}

if (!isset($record['maildigest'])) {
$record['maildigest'] = $CFG->defaultpreference_maildigest;
}

if (!isset($record['autosubscribe'])) {
$record['autosubscribe'] = $CFG->defaultpreference_autosubscribe;
}

if (!isset($record['trackforums'])) {
$record['trackforums'] = $CFG->defaultpreference_trackforums;
}

if (!isset($record['deleted'])) {
$record['deleted'] = 0;
}

if (!isset($record['timecreated'])) {
$record['timecreated'] = time();
}

$record['timemodified'] = $record['timecreated'];

if (!isset($record['lastip'])) {
$record['lastip'] = '0.0.0.0';
}

if ($record['deleted']) {
$delname = $record['email'].'.'.time();
while ($DB->record_exists('user', array('username'=>$delname))) {
$delname++;
}
$record['idnumber'] = '';
$record['email'] = md5($record['username']);
$record['username'] = $delname;
$record['picture'] = 0;
}
$tobedeleted = !empty($record['deleted']);
unset($record['deleted']);

$userid = $DB->insert_record('user', $record);
$userid = user_create_user($record, false, false);

if (!$record['deleted']) {
context_user::instance($userid);
if ($extrafields = array_intersect_key($record, ['password' => 1, 'timecreated' => 1])) {
$DB->update_record('user', ['id' => $userid] + $extrafields);
}

if (!$tobedeleted) {
// All new not deleted users must have a favourite self-conversation.
$selfconversation = \core_message\api::create_conversation(
\core_message\api::MESSAGE_CONVERSATION_TYPE_SELF,
[$userid]
);
\core_message\api::set_favourite_conversation($selfconversation->id, $userid);

// Save custom profile fields data.
$hasprofilefields = array_filter($record, function($key){
return strpos($key, 'profile_field_') === 0;
}, ARRAY_FILTER_USE_KEY);
if ($hasprofilefields) {
require_once($CFG->dirroot.'/user/profile/lib.php');
$usernew = (object)(['id' => $userid] + $record);
profile_save_data($usernew);
}
}

$user = $DB->get_record('user', array('id' => $userid), '*', MUST_EXIST);

if (!$record['deleted'] && isset($record['interests'])) {
if (!$tobedeleted && isset($record['interests'])) {
require_once($CFG->dirroot . '/user/editlib.php');
if (!is_array($record['interests'])) {
$record['interests'] = preg_split('/\s*,\s*/', trim($record['interests']), -1, PREG_SPLIT_NO_EMPTY);
}
useredit_update_interests($user, $record['interests']);
}

\core\event\user_created::create_from_userid($userid)->trigger();

if ($tobedeleted) {
delete_user($user);
$user = $DB->get_record('user', array('id' => $userid));
}
return $user;
}

Expand Down
4 changes: 1 addition & 3 deletions lib/testing/tests/generator_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ public function test_create_user() {
$this->assertEquals($count + 1, $DB->count_records('user'));
$this->assertSame($user->username, core_user::clean_field($user->username, 'username'));
$this->assertSame($user->email, core_user::clean_field($user->email, 'email'));
$this->assertSame(AUTH_PASSWORD_NOT_CACHED, $user->password);
$this->assertNotEmpty($user->firstnamephonetic);
$this->assertNotEmpty($user->lastnamephonetic);
$this->assertNotEmpty($user->alternatename);
Expand All @@ -97,7 +96,6 @@ public function test_create_user() {
'password' => 'password1',
'email' => '[email protected]',
'confirmed' => '1',
'lang' => 'cs',
'maildisplay' => '1',
'mailformat' => '0',
'maildigest' => '1',
Expand Down Expand Up @@ -128,7 +126,7 @@ public function test_create_user() {
$this->assertEquals($count + 3, $DB->count_records('user'));
$this->assertSame('', $user->idnumber);
$this->assertSame(md5($record['username']), $user->email);
$this->assertFalse(context_user::instance($user->id, IGNORE_MISSING));
$this->assertEquals(1, $user->deleted);

// Test generating user with interests.
$user = $generator->create_user(array('interests' => 'Cats, Dogs'));
Expand Down
2 changes: 0 additions & 2 deletions lib/tests/accesslib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2749,8 +2749,6 @@ public function test_permission_evaluation() {
$bi = $generator->create_block('online_users', array('parentcontextid'=>$usercontext->id));
$testblocks[] = $bi->id;
}
// Deleted user - should be ignored everywhere, can not have context.
$generator->create_user(array('deleted'=>1));

// Add block to frontpage.
$bi = $generator->create_block('online_users', array('parentcontextid'=>$frontpagecontext->id));
Expand Down
8 changes: 5 additions & 3 deletions lib/tests/moodlelib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2862,10 +2862,12 @@ public function test_update_internal_user_password() {
* the user table and fire event.
*/
public function test_update_internal_user_password_no_cache() {
global $DB;
$this->resetAfterTest();

$user = $this->getDataGenerator()->create_user(array('auth' => 'cas'));
$this->assertEquals(AUTH_PASSWORD_NOT_CACHED, $user->password);
$DB->update_record('user', ['id' => $user->id, 'password' => AUTH_PASSWORD_NOT_CACHED]);
$user->password = AUTH_PASSWORD_NOT_CACHED;

$sink = $this->redirectEvents();
update_internal_user_password($user, 'wonkawonka');
Expand Down Expand Up @@ -3594,7 +3596,7 @@ public function test_generate_confirmation_link($username, $confirmationurl, $ex
$user = $this->getDataGenerator()->create_user(
[
"username" => $username,
"confirmed" => false,
"confirmed" => 0,
"email" => '[email protected]',
]
);
Expand Down Expand Up @@ -3623,7 +3625,7 @@ public function test_generate_confirmation_link_with_custom_admin() {
$user = $this->getDataGenerator()->create_user(
[
"username" => "many_-.@characters@[email protected]..",
"confirmed" => false,
"confirmed" => 0,
"email" => '[email protected]',
]
);
Expand Down
32 changes: 23 additions & 9 deletions lib/tests/outputcomponents_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,17 @@ public function test_get_url() {
$user2 = $this->getDataGenerator()->create_user(array('picture'=>0, 'email'=>'[email protected]'));
$context2 = context_user::instance($user2->id);

// User 3 is deleted.
$user3 = $this->getDataGenerator()->create_user(array('picture'=>1, 'deleted'=>1, 'email'=>'[email protected]'));
$context3 = context_user::instance($user3->id, IGNORE_MISSING);
$this->assertNotEmpty(context_user::instance($user3->id));
$this->assertEquals(0, $user3->picture);
$this->assertNotEquals('[email protected]', $user3->email);
$this->assertFalse($context3);

// User 4 is incorrectly deleted with its context deleted as well (testing legacy code).
$user4 = $this->getDataGenerator()->create_user(['picture' => 1, 'deleted' => 1, 'email' => '[email protected]']);
context_helper::delete_instance(CONTEXT_USER, $user4->id);
$this->assertEquals(0, $user4->picture);
$this->assertNotEquals('[email protected]', $user4->email);

// Try legacy picture == 1.
$user1->picture = 1;
Expand Down Expand Up @@ -186,13 +192,14 @@ public function test_get_url() {
$this->assertSame($CFG->wwwroot.'/theme/image.php/boost/core/1/u/f2', $up3->get_url($page, $renderer)->out(false));
$this->assertEquals($reads, $DB->perf_get_reads());

// Try incorrectly deleted users (with valid email and pciture flag) - some DB reads expected.
$user3->email = '[email protected]';
$user3->picture = 1;
// Try incorrectly deleted users (with valid email and picture flag, but user context removed) - some DB reads expected.
unset($user4->deleted);
$user4->email = '[email protected]';
$user4->picture = 1;
$reads = $DB->perf_get_reads();
$up3 = new user_picture($user3);
$up4 = new user_picture($user4);
$this->assertEquals($reads, $DB->perf_get_reads());
$this->assertSame($CFG->wwwroot.'/theme/image.php/boost/core/1/u/f2', $up3->get_url($page, $renderer)->out(false));
$this->assertSame($CFG->wwwroot.'/theme/image.php/boost/core/1/u/f2', $up4->get_url($page, $renderer)->out(false));
$this->assertGreaterThan($reads, $DB->perf_get_reads());

// Test gravatar.
Expand All @@ -203,6 +210,10 @@ public function test_get_url() {
$user3->picture = 0;
$up3 = new user_picture($user3);
$this->assertSame($CFG->wwwroot.'/theme/image.php/boost/core/1/u/f2', $up3->get_url($page, $renderer)->out(false));
$user4->email = 'deleted';
$user4->picture = 0;
$up4 = new user_picture($user4);
$this->assertSame($CFG->wwwroot.'/theme/image.php/boost/core/1/u/f2', $up4->get_url($page, $renderer)->out(false));

// Http version.
$CFG->wwwroot = str_replace('https:', 'http:', $CFG->wwwroot);
Expand Down Expand Up @@ -237,11 +248,14 @@ public function test_get_url() {
$up1 = new user_picture($user1);
$this->assertSame($CFG->wwwroot.'/pluginfile.php/'.$context1->id.'/user/icon/boost/f2?rev=11', $up1->get_url($page, $renderer)->out(false));

$up2 = new user_picture($user2);
$this->assertSame('https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?s=35&d=https%3A%2F%2Fwww.example.com%2Fmoodle%2Fpix%2Fu%2Ff2.png', $up2->get_url($page, $renderer)->out(false));

$up3 = new user_picture($user3);
$this->assertSame($CFG->wwwroot.'/theme/image.php/boost/core/1/u/f2', $up3->get_url($page, $renderer)->out(false));

$up2 = new user_picture($user2);
$this->assertSame('https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?s=35&d=https%3A%2F%2Fwww.example.com%2Fmoodle%2Fpix%2Fu%2Ff2.png', $up2->get_url($page, $renderer)->out(false));
$up4 = new user_picture($user4);
$this->assertSame($CFG->wwwroot.'/theme/image.php/boost/core/1/u/f2', $up4->get_url($page, $renderer)->out(false));

// TODO MDL-44792 Rewrite those tests to use a fixture.
// Now test gravatar with one theme having own images (afterburner).
Expand Down
1 change: 1 addition & 0 deletions lib/upgrade.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ information provided here is intended especially for developers.
This hook allow plugin developers to add information that is displayed on category deletion form. Function should
return string, which will be added to the list of category contents shown on the form. $category param is an instance
of core_course_category class.
* Data generator create_user in both unittests and behat now validates user fields and triggers user_created event

=== 3.8 ===
* Add CLI option to notify all cron tasks to stop: admin/cli/cron.php --stop
Expand Down
10 changes: 3 additions & 7 deletions user/tests/externallib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,6 @@ public function test_get_users_by_field() {
'city' => 'Perth',
'url' => 'http://moodle.org',
'country' => 'AU',
'lang' => 'kkl',
'theme' => 'kkt',
);
$user1 = self::getDataGenerator()->create_user($user1);
if (!empty($CFG->usetags)) {
Expand Down Expand Up @@ -330,11 +328,9 @@ public function test_get_users_by_field() {
if (!empty($CFG->usetags) and !empty($generateduser->interests)) {
$this->assertEquals(implode(', ', $generateduser->interests), $returneduser['interests']);
}
// Check empty since incorrect values were used when creating the user.
if ($returneduser['id'] == $user1->id) {
$this->assertEmpty($returneduser['lang']);
$this->assertEmpty($returneduser['theme']);
}
// Default language and no theme were used for the user.
$this->assertEquals($CFG->lang, $returneduser['lang']);
$this->assertEmpty($returneduser['theme']);
}
}

Expand Down
8 changes: 4 additions & 4 deletions user/tests/privacy_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public function test_delete_data_for_all_users_in_context() {
'institution' => 'test',
'department' => 'Science',
'city' => 'Perth',
'country' => 'au'
'country' => 'AU'
]);
$user2 = $this->getDataGenerator()->create_user();
$course = $this->getDataGenerator()->create_course();
Expand Down Expand Up @@ -222,7 +222,7 @@ public function test_delete_data_for_user() {
'institution' => 'test',
'department' => 'Science',
'city' => 'Perth',
'country' => 'au'
'country' => 'AU'
]);
$user2 = $this->getDataGenerator()->create_user();
$course = $this->getDataGenerator()->create_course();
Expand Down Expand Up @@ -328,7 +328,7 @@ public function test_delete_data_for_users() {
'institution' => 'test',
'department' => 'Science',
'city' => 'Perth',
'country' => 'au'
'country' => 'AU'
]);
$usercontext1 = \context_user::instance($user1->id);
$userlist1 = new \core_privacy\local\request\userlist($usercontext1, $component);
Expand All @@ -342,7 +342,7 @@ public function test_delete_data_for_users() {
'institution' => 'test',
'department' => 'Science',
'city' => 'Perth',
'country' => 'au'
'country' => 'AU'
]);
$usercontext2 = \context_user::instance($user2->id);
$userlist2 = new \core_privacy\local\request\userlist($usercontext2, $component);
Expand Down
30 changes: 30 additions & 0 deletions user/tests/profilelib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,4 +210,34 @@ public function test_profile_has_required_custom_fields_set() {
$this->assertTrue(user_not_fully_set_up($roaminghermione, true));
$this->assertTrue(user_not_fully_set_up($roaminghermione, false));
}

/**
* Test that user generator sets the custom profile fields
*/
public function test_profile_fields_in_generator() {
global $CFG, $DB;
require_once($CFG->dirroot.'/mnet/lib.php');

$this->resetAfterTest();

// Add a required, visible, unlocked custom field.
$DB->insert_record('user_info_field', ['shortname' => 'house', 'name' => 'House', 'required' => 1,
'visible' => 1, 'locked' => 0, 'categoryid' => 1, 'datatype' => 'text']);

// Create some student accounts.
$hermione = $this->getDataGenerator()->create_user(['profile_field_house' => 'Gryffindor']);
$harry = $this->getDataGenerator()->create_user();

// Only students with required fields filled should be considered as fully set up.
$this->assertFalse(user_not_fully_set_up($hermione));
$this->assertTrue(user_not_fully_set_up($harry));

// Test that the profile fields were actually set.
$profilefields1 = profile_user_record($hermione->id);
$this->assertEquals('Gryffindor', $profilefields1->house);

$profilefields2 = profile_user_record($harry->id);
$this->assertObjectHasAttribute('house', $profilefields2);
$this->assertNull($profilefields2->house);
}
}
Loading

0 comments on commit 1c1fd15

Please sign in to comment.