diff --git a/enrol/manual/tests/behat/quickenrolment.feature b/enrol/manual/tests/behat/quickenrolment.feature index 6d4494fe960..49e51ba14ab 100644 --- a/enrol/manual/tests/behat/quickenrolment.feature +++ b/enrol/manual/tests/behat/quickenrolment.feature @@ -173,7 +173,7 @@ 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 @@ -181,7 +181,7 @@ Feature: Teacher can search and enrol users one by one into the course And I press "Enrol users" When I set the field "Select users" to "student100@example.com" And I click on ".form-autocomplete-downarrow" "css_element" in the "Select users" "form_row" - Then I should see "student100@example.com, CITY1, UK, 1234567892, 1234567893, ABC1, ABC2" + Then I should see "student100@example.com, 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 | diff --git a/grade/tests/report_graderlib_test.php b/grade/tests/report_graderlib_test.php index 152fc3b2ec1..d617f487cbe 100644 --- a/grade/tests/report_graderlib_test.php +++ b/grade/tests/report_graderlib_test.php @@ -47,7 +47,7 @@ class core_grade_report_graderlib_testcase extends advanced_testcase { $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); diff --git a/grade/tests/reportuserlib_test.php b/grade/tests/reportuserlib_test.php index 94ae2925e72..301b6bdad60 100644 --- a/grade/tests/reportuserlib_test.php +++ b/grade/tests/reportuserlib_test.php @@ -56,11 +56,11 @@ class core_grade_reportuserlib_testcase extends advanced_testcase { $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); diff --git a/lib/testing/generator/data_generator.php b/lib/testing/generator/data_generator.php index 2e46b8b4377..c8e031d6c2a 100644 --- a/lib/testing/generator/data_generator.php +++ b/lib/testing/generator/data_generator.php @@ -142,6 +142,7 @@ EOD; */ public function create_user($record=null, array $options=null) { global $DB, $CFG; + require_once($CFG->dirroot.'/user/lib.php'); $this->usercounter++; $i = $this->usercounter; @@ -206,10 +207,6 @@ EOD; 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'])) { @@ -220,71 +217,41 @@ EOD; $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 = user_create_user($record, false, false); + + if ($extrafields = array_intersect_key($record, ['password' => 1, 'timecreated' => 1])) { + $DB->update_record('user', ['id' => $userid] + $extrafields); } - $userid = $DB->insert_record('user', $record); - - if (!$record['deleted']) { - context_user::instance($userid); - + 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); @@ -292,6 +259,12 @@ EOD; 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; } diff --git a/lib/testing/tests/generator_test.php b/lib/testing/tests/generator_test.php index d22a9101a4a..2d9990fae25 100644 --- a/lib/testing/tests/generator_test.php +++ b/lib/testing/tests/generator_test.php @@ -70,7 +70,6 @@ class core_test_generator_testcase extends advanced_testcase { $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); @@ -97,7 +96,6 @@ class core_test_generator_testcase extends advanced_testcase { 'password' => 'password1', 'email' => 'email@example.com', 'confirmed' => '1', - 'lang' => 'cs', 'maildisplay' => '1', 'mailformat' => '0', 'maildigest' => '1', @@ -128,7 +126,7 @@ class core_test_generator_testcase extends advanced_testcase { $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')); diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index dcf3646c10a..0382f72e8e5 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -2749,8 +2749,6 @@ class core_accesslib_testcase extends advanced_testcase { $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)); diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index fbfbbb6850d..1648a8fb9fb 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -2862,10 +2862,12 @@ class core_moodlelib_testcase extends advanced_testcase { * 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'); @@ -3594,7 +3596,7 @@ class core_moodlelib_testcase extends advanced_testcase { $user = $this->getDataGenerator()->create_user( [ "username" => $username, - "confirmed" => false, + "confirmed" => 0, "email" => 'test@example.com', ] ); @@ -3623,7 +3625,7 @@ class core_moodlelib_testcase extends advanced_testcase { $user = $this->getDataGenerator()->create_user( [ "username" => "many_-.@characters@_@-..-..", - "confirmed" => false, + "confirmed" => 0, "email" => 'test@example.com', ] ); diff --git a/lib/tests/outputcomponents_test.php b/lib/tests/outputcomponents_test.php index 0a631aa1345..a5e1515f06c 100644 --- a/lib/tests/outputcomponents_test.php +++ b/lib/tests/outputcomponents_test.php @@ -140,11 +140,17 @@ class core_outputcomponents_testcase extends advanced_testcase { $user2 = $this->getDataGenerator()->create_user(array('picture'=>0, 'email'=>'user2@example.com')); $context2 = context_user::instance($user2->id); + // User 3 is deleted. $user3 = $this->getDataGenerator()->create_user(array('picture'=>1, 'deleted'=>1, 'email'=>'user3@example.com')); - $context3 = context_user::instance($user3->id, IGNORE_MISSING); + $this->assertNotEmpty(context_user::instance($user3->id)); $this->assertEquals(0, $user3->picture); $this->assertNotEquals('user3@example.com', $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' => 'user4@example.com']); + context_helper::delete_instance(CONTEXT_USER, $user4->id); + $this->assertEquals(0, $user4->picture); + $this->assertNotEquals('user4@example.com', $user4->email); // Try legacy picture == 1. $user1->picture = 1; @@ -186,13 +192,14 @@ class core_outputcomponents_testcase extends advanced_testcase { $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 = 'user3@example.com'; - $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 = 'user4@example.com'; + $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. @@ -203,6 +210,10 @@ class core_outputcomponents_testcase extends advanced_testcase { $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); @@ -237,11 +248,14 @@ class core_outputcomponents_testcase extends advanced_testcase { $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). diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 36a0bb464d8..4e79b930750 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -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 diff --git a/user/tests/externallib_test.php b/user/tests/externallib_test.php index fde87863b68..71de5348a70 100644 --- a/user/tests/externallib_test.php +++ b/user/tests/externallib_test.php @@ -223,8 +223,6 @@ class core_user_externallib_testcase extends externallib_advanced_testcase { 'city' => 'Perth', 'url' => 'http://moodle.org', 'country' => 'AU', - 'lang' => 'kkl', - 'theme' => 'kkt', ); $user1 = self::getDataGenerator()->create_user($user1); if (!empty($CFG->usetags)) { @@ -330,11 +328,9 @@ class core_user_externallib_testcase extends externallib_advanced_testcase { 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']); } } diff --git a/user/tests/privacy_test.php b/user/tests/privacy_test.php index cc5e6ed43aa..cedc5658bd3 100644 --- a/user/tests/privacy_test.php +++ b/user/tests/privacy_test.php @@ -151,7 +151,7 @@ class core_user_privacy_testcase extends provider_testcase { 'institution' => 'test', 'department' => 'Science', 'city' => 'Perth', - 'country' => 'au' + 'country' => 'AU' ]); $user2 = $this->getDataGenerator()->create_user(); $course = $this->getDataGenerator()->create_course(); @@ -222,7 +222,7 @@ class core_user_privacy_testcase extends provider_testcase { 'institution' => 'test', 'department' => 'Science', 'city' => 'Perth', - 'country' => 'au' + 'country' => 'AU' ]); $user2 = $this->getDataGenerator()->create_user(); $course = $this->getDataGenerator()->create_course(); @@ -328,7 +328,7 @@ class core_user_privacy_testcase extends provider_testcase { '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); @@ -342,7 +342,7 @@ class core_user_privacy_testcase extends provider_testcase { '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); diff --git a/user/tests/profilelib_test.php b/user/tests/profilelib_test.php index 0c4c5993dac..59d8b5dc603 100644 --- a/user/tests/profilelib_test.php +++ b/user/tests/profilelib_test.php @@ -210,4 +210,34 @@ class core_user_profilelib_testcase extends advanced_testcase { $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); + } } diff --git a/user/tests/userlib_test.php b/user/tests/userlib_test.php index c8327f3e235..28c37fd097c 100644 --- a/user/tests/userlib_test.php +++ b/user/tests/userlib_test.php @@ -833,23 +833,23 @@ class core_userliblib_testcase extends advanced_testcase { $this->resetAfterTest(true); $this->setAdminUser(); // We need capabilities to view the data. $user = self::getDataGenerator()->create_user([ - 'auth' => 'auth_something', + 'auth' => 'email', 'confirmed' => '0', 'idnumber' => 'someidnumber', 'lang' => 'en', 'theme' => $CFG->theme, - 'timezone' => '50', + 'timezone' => '5', 'mailformat' => '0', ]); // Fields that should get by default. $got = user_get_user_details($user); - self::assertSame('auth_something', $got['auth']); + self::assertSame('email', $got['auth']); self::assertSame('0', $got['confirmed']); self::assertSame('someidnumber', $got['idnumber']); self::assertSame('en', $got['lang']); self::assertSame($CFG->theme, $got['theme']); - self::assertSame('50', $got['timezone']); + self::assertSame('5', $got['timezone']); self::assertSame('0', $got['mailformat']); }