Merge branch 'MDL-68333-master' of git://github.com/marinaglancy/moodle

diff --git a/enrol/manual/tests/behat/quickenrolment.feature b/enrol/manual/tests/behat/quickenrolment.feature
index 6d4494f..49e51ba 100644 (file)
--- 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 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 36a0bb4..4e79b93 100644 (file)
--- 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 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']);
     }