MDL-68333 testing: trigger user_created event in user generator

diff --git a/enrol/manual/tests/behat/quickenrolment.feature b/enrol/manual/tests/behat/quickenrolment.feature
index f2f3f6c..cd3c979 100644 (file)
--- a/enrol/manual/tests/behat/quickenrolment.feature
+++ b/enrol/manual/tests/behat/quickenrolment.feature
@@ -171,7 +171,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
@@ -179,7 +179,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 152fc3b..d617f48 100644 (file)
--- 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 94ae292..301b6bd 100644 (file)
--- 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 2e46b8b..c8e031d 100644 (file)
--- 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 = $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);
@@ -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 d22a910..2d9990f 100644 (file)
--- 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 dcf3646..0382f72 100644 (file)
--- 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 fbfbbb6..1648a8f 100644 (file)
--- 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 0a631aa..a5e1515 100644 (file)
--- 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 c6e0d0d..ec9b975 100644 (file)
--- a/lib/upgrade.txt
+++ b/lib/upgrade.txt
@@ -71,6 +71,7 @@ information provided here is intended especially for developers.
   - cron_execute_plugin_type()
   - cron_bc_hack_plugin_functions()
   Please, use the Task API instead: https://docs.moodle.org/dev/Task_API
+* 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 fde8786..71de534 100644 (file)
--- 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 cc5e6ed..cedc565 100644 (file)
--- 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 0c4c599..59d8b5d 100644 (file)
--- 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 c8327f3..28c37fd 100644 (file)
--- 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']);
     }