Merge branch 'MDL-63147-master' of git://github.com/peterRd/moodle
authorJake Dallimore <jake@moodle.com>
Wed, 12 Dec 2018 00:25:02 +0000 (08:25 +0800)
committerJake Dallimore <jake@moodle.com>
Wed, 12 Dec 2018 00:25:02 +0000 (08:25 +0800)
grade/edit/outcome/course_form.html
message/amd/build/message_drawer_view_overview_section.min.js
message/amd/src/message_drawer_view_overview_section.js
mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php
mod/lti/service/gradebookservices/tests/task_cleanup_test.php
privacy/classes/local/request/moodle_content_writer.php
question/question.php
question/tests/behat/copy_questions.feature
user/externallib.php
user/tests/externallib_test.php

index 3eb01c9..baf056a 100644 (file)
@@ -7,7 +7,7 @@
         <td>
             <label for="removeoutcomes"><?php print_string('outcomescourse', 'grades'); ?></label>
             <br />
-            <select id="removeoutcomes" size="20" name="removeoutcomes[]" multiple="multiple">
+            <select id="removeoutcomes" size="20" name="removeoutcomes[]" multiple="multiple" class="form-control input-block-level">
             <?php
             if ($co_standard_notused) {
                 echo '<optgroup label="'.get_string('outcomescoursenotused', 'grades').'">';
         <?php
         if (has_capability('moodle/grade:manageoutcomes', $context)) {
         ?>
-        <td>
+        <td class="p-l-1 p-r-1">
             <p class="arrow_button">
-                <input name="add" id="add" type="submit" value="<?php echo '&nbsp; '.$OUTPUT->larrow().' &nbsp; &nbsp; '.get_string('add'); ?>" title="<?php print_string('add'); ?>" />
+                <input name="add" class="btn btn_secondary" id="add" type="submit" value="<?php echo '&nbsp; ' . $OUTPUT->larrow() . ' &nbsp; &nbsp; ' .
+                    get_string('add'); ?>" title="<?php print_string('add'); ?>" />
                 <br />
-                <input name="remove" id="remove" type="submit" value="<?php echo '&nbsp;'.$OUTPUT->rarrow().' &nbsp; &nbsp; '.get_string('remove'); ?>" title="<?php print_string('remove'); ?>" />
+                <input name="remove" class="btn btn_secondary" id="remove" type="submit" value="<?php echo '&nbsp;' . get_string('remove') . ' &nbsp; &nbsp; ' .
+                    $OUTPUT->rarrow(); ?>" title="<?php print_string('remove'); ?>" />
             </p>
         </td>
         <?php } ?>
         <td>
             <label for="addoutcomes"><?php print_string('outcomesstandardavailable', 'grades'); ?></label>
             <br />
-            <select id="addoutcomes" size="20" name="addoutcomes[]" multiple="multiple">
+            <select id="addoutcomes" size="20" name="addoutcomes[]" multiple="multiple" class="form-control input-block-level">
 
             <?php
             foreach ($standardoutcomes as $outcome) {
index d33358a..0535a9c 100644 (file)
Binary files a/message/amd/build/message_drawer_view_overview_section.min.js and b/message/amd/build/message_drawer_view_overview_section.min.js differ
index a27bfc2..45e5f76 100644 (file)
@@ -625,10 +625,11 @@ function(
                 // Silently ignore if we can't updated the counts. No need to bother the user.
             });
 
-            // Same as for total counts.
+            // This is given to us by the calling code because the unread counts for all sections
+            // are loaded in a single ajax request rather than one request per section.
             unreadCountPromise.then(function(count) {
                 renderUnreadCount(root, count);
-                loadedTotalCounts = true;
+                loadedUnreadCounts = true;
                 return;
             })
             .catch(function() {
index 554f525..9c7783e 100644 (file)
@@ -546,9 +546,7 @@ class gradebookservices extends service_base {
         $sql = "DELETE
                   FROM {ltiservice_gradebookservices}
                  WHERE gradeitemid NOT IN (SELECT id
-                                             FROM {grade_items} gi
-                                            WHERE gi.itemtype = 'mod'
-                                              AND gi.itemmodule = 'lti')";
+                                             FROM {grade_items} gi)";
         $DB->execute($sql);
     }
 
index 20881e0..c9ed004 100644 (file)
@@ -100,4 +100,46 @@ class ltiservice_gradebookservices_cleanup_task_testcase extends advanced_testca
 
         $this->assertEquals($gradeitem2->id, $gradebookserviceitem->gradeitemid);
     }
+
+    /**
+     * Test the cleanup task with a manual grade item.
+     */
+    public function test_cleanup_task_with_manual_item() {
+        global $CFG, $DB;
+
+        // This is required when running the unit test in isolation.
+        require_once($CFG->libdir . '/gradelib.php');
+
+        // Create a manual grade item for a course.
+        $course = $this->getDataGenerator()->create_course();
+        $params = [
+            'courseid' => $course->id,
+            'itemtype' => 'manual'
+        ];
+        $gradeitem = new grade_item($params);
+        $gradeitem->insert();
+
+        // Insert it into the 'ltiservice_gradebookservices' table.
+        $data = new stdClass();
+        $data->gradeitemid = $gradeitem->id;
+        $data->courseid = $course->id;
+        $DB->insert_record('ltiservice_gradebookservices', $data);
+
+        // Run the task.
+        $task = new \ltiservice_gradebookservices\task\cleanup_task();
+        $task->execute();
+
+        // Check it still exist.
+        $this->assertEquals(1, $DB->count_records('ltiservice_gradebookservices'));
+
+        // Delete the manual item.
+        $gradeitem->delete();
+
+        // Run the task again.
+        $task = new \ltiservice_gradebookservices\task\cleanup_task();
+        $task->execute();
+
+        // Check it has been removed.
+        $this->assertEquals(0, $DB->count_records('ltiservice_gradebookservices'));
+    }
 }
index 5ba00a6..327cd9e 100644 (file)
@@ -212,7 +212,8 @@ class moodle_content_writer implements content_writer {
                 [$file->get_filepath()]
             );
             $path = $this->get_path($pathitems, $file->get_filename());
-            check_dir_exists(dirname($path), true, true);
+            $fullpath = $this->get_full_path($pathitems, $file->get_filename());
+            check_dir_exists(dirname($fullpath), true, true);
             $this->files[$path] = $file;
         }
 
index 0f78fc7..a37d419 100644 (file)
@@ -178,6 +178,7 @@ if ($id) {
     if ($makecopy) {
         // If we are duplicating a question, add some indication to the question name.
         $question->name = get_string('questionnamecopy', 'question', $question->name);
+        $question->idnumber = null;
         $question->beingcopied = true;
     }
 
index 7ae3708..e6e6096 100644 (file)
@@ -18,8 +18,8 @@ Feature: A teacher can duplicate questions in the question bank
       | contextlevel | reference | name           |
       | Course       | C1        | Test questions |
     And the following "questions" exist:
-      | questioncategory | qtype | name                       | questiontext                  |
-      | Test questions   | essay | Test question to be copied | Write about whatever you want |
+      | questioncategory | qtype | name                       | questiontext                  | idnumber |
+      | Test questions   | essay | Test question to be copied | Write about whatever you want | qid      |
     And I log in as "teacher1"
     And I am on "Course 1" course homepage
     And I navigate to "Question bank > Questions" in current page administration
index 2023cd8..84adf0f 100644 (file)
@@ -161,7 +161,6 @@ class core_user_external extends external_api {
         $transaction = $DB->start_delegated_transaction();
 
         $userids = array();
-        $createpassword = false;
         foreach ($params['users'] as $user) {
             // Make sure that the username, firstname and lastname are not blank.
             foreach (array('username', 'firstname', 'lastname') as $fieldname) {
@@ -194,7 +193,8 @@ class core_user_external extends external_api {
             }
 
             // Make sure we have a password or have to create one.
-            if (empty($user['password']) && empty($user['createpassword'])) {
+            $authplugin = get_auth_plugin($user['auth']);
+            if ($authplugin->is_internal() && empty($user['password']) && empty($user['createpassword'])) {
                 throw new invalid_parameter_exception('Invalid password: you must provide a password, or set createpassword.');
             }
 
@@ -213,11 +213,15 @@ class core_user_external extends external_api {
 
             $createpassword = !empty($user['createpassword']);
             unset($user['createpassword']);
-            if ($createpassword) {
-                $user['password'] = '';
-                $updatepassword = false;
+            $updatepassword = false;
+            if ($authplugin->is_internal()) {
+                if ($createpassword) {
+                    $user['password'] = '';
+                } else {
+                    $updatepassword = true;
+                }
             } else {
-                $updatepassword = true;
+                $user['password'] = AUTH_PASSWORD_NOT_CACHED;
             }
 
             // Create the user data now!
index 36fa7b5..02891df 100644 (file)
@@ -484,7 +484,7 @@ class core_user_externallib_testcase extends externallib_advanced_testcase {
      * Test create_users
      */
     public function test_create_users() {
-         global $USER, $CFG, $DB;
+        global $DB;
 
         $this->resetAfterTest(true);
 
@@ -517,46 +517,85 @@ class core_user_externallib_testcase extends externallib_advanced_testcase {
             'interests' => 'badminton, basketball, cooking,  '
         );
 
+        // User with an authentication method done externally.
+        $user2 = array(
+            'username' => 'usernametest2',
+            'firstname' => 'First Name User Test 2',
+            'lastname' => 'Last Name User Test 2',
+            'email' => 'usertest2@example.com',
+            'auth' => 'oauth2'
+        );
+
         $context = context_system::instance();
         $roleid = $this->assignUserCapability('moodle/user:create', $context->id);
         $this->assignUserCapability('moodle/user:editprofile', $context->id, $roleid);
 
         // Call the external function.
-        $createdusers = core_user_external::create_users(array($user1));
+        $createdusers = core_user_external::create_users(array($user1, $user2));
 
         // We need to execute the return values cleaning process to simulate the web service server.
         $createdusers = external_api::clean_returnvalue(core_user_external::create_users_returns(), $createdusers);
 
         // Check we retrieve the good total number of created users + no error on capability.
-        $this->assertEquals(1, count($createdusers));
+        $this->assertCount(2, $createdusers);
 
         foreach($createdusers as $createduser) {
             $dbuser = $DB->get_record('user', array('id' => $createduser['id']));
-            $this->assertEquals($dbuser->username, $user1['username']);
-            $this->assertEquals($dbuser->idnumber, $user1['idnumber']);
-            $this->assertEquals($dbuser->firstname, $user1['firstname']);
-            $this->assertEquals($dbuser->lastname, $user1['lastname']);
-            $this->assertEquals($dbuser->email, $user1['email']);
-            $this->assertEquals($dbuser->description, $user1['description']);
-            $this->assertEquals($dbuser->city, $user1['city']);
-            $this->assertEquals($dbuser->country, $user1['country']);
-            $this->assertEquals($dbuser->department, $user1['department']);
-            $this->assertEquals($dbuser->institution, $user1['institution']);
-            $this->assertEquals($dbuser->phone1, $user1['phone1']);
-            $this->assertEquals($dbuser->maildisplay, $user1['maildisplay']);
-            $this->assertEquals('atto', get_user_preferences('htmleditor', null, $dbuser));
-            $this->assertEquals(null, get_user_preferences('invalidpreference', null, $dbuser));
-            // Confirm user interests have been saved.
-            $interests = core_tag_tag::get_item_tags_array('core', 'user', $createduser['id'], core_tag_tag::BOTH_STANDARD_AND_NOT,
-                0, false);
-            // There should be 3 user interests.
-            $this->assertCount(3, $interests);
+
+            if ($createduser['username'] === $user1['username']) {
+                $usertotest = $user1;
+                $this->assertEquals('atto', get_user_preferences('htmleditor', null, $dbuser));
+                $this->assertEquals(null, get_user_preferences('invalidpreference', null, $dbuser));
+                // Confirm user interests have been saved.
+                $interests = core_tag_tag::get_item_tags_array('core', 'user', $createduser['id'],
+                        core_tag_tag::BOTH_STANDARD_AND_NOT, 0, false);
+                // There should be 3 user interests.
+                $this->assertCount(3, $interests);
+
+            } else if ($createduser['username'] === $user2['username']) {
+                $usertotest = $user2;
+            }
+
+            foreach ($dbuser as $property => $value) {
+                if ($property === 'password') {
+                    if ($usertotest === $user2) {
+                        // External auth mechanisms don't store password in the user table.
+                        $this->assertEquals(AUTH_PASSWORD_NOT_CACHED, $value);
+                    } else {
+                        // Skip hashed passwords.
+                        continue;
+                    }
+                }
+                // Confirm that the values match.
+                if (isset($usertotest[$property])) {
+                    $this->assertEquals($usertotest[$property], $value);
+                }
+            }
         }
 
         // Call without required capability
         $this->unassignUserCapability('moodle/user:create', $context->id, $roleid);
         $this->expectException('required_capability_exception');
-        $createdusers = core_user_external::create_users(array($user1));
+        core_user_external::create_users(array($user1));
+    }
+
+    /**
+     * Test create_users with password and createpassword parameter not set.
+     */
+    public function test_create_users_empty_password() {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $user = [
+            'username' => 'usernametest1',
+            'firstname' => 'First Name User Test 1',
+            'lastname' => 'Last Name User Test 1',
+            'email' => 'usertest1@example.com',
+        ];
+
+        // This should throw an exception because either password or createpassword param must be passed for auth_manual.
+        $this->expectException(invalid_parameter_exception::class);
+        core_user_external::create_users([$user]);
     }
 
     /**