Merge branch 'MDL-64429' of https://github.com/paulholden/moodle
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Thu, 26 Mar 2020 17:49:36 +0000 (18:49 +0100)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Thu, 26 Mar 2020 17:49:36 +0000 (18:49 +0100)
47 files changed:
admin/settings/subsystems.php
admin/tool/dataprivacy/tests/privacy_provider_test.php [new file with mode: 0644]
backup/util/helper/async_helper.class.php
filter/displayh5p/db/upgrade.php
filter/displayh5p/lang/en/filter_displayh5p.php
filter/displayh5p/settings.php
filter/displayh5p/version.php
h5p/h5plib/v124/joubel/core/h5p.classes.php
h5p/h5plib/v124/joubel/core/readme_moodle.txt
lang/en/admin.php
lib/authlib.php
lib/editor/atto/plugins/h5p/lang/en/atto_h5p.php
lib/editor/atto/plugins/h5p/lang/en/deprecated.txt [new file with mode: 0644]
lib/editor/atto/plugins/h5p/lib.php
lib/editor/atto/plugins/h5p/tests/behat/h5p.feature
lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button-debug.js
lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button-min.js
lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button.js
lib/editor/atto/plugins/h5p/yui/src/button/js/button.js
lib/moodlelib.php
lib/outputrenderers.php
lib/pagelib.php
lib/questionlib.php
lib/tests/authlib_test.php
lib/tests/questionlib_test.php
login/lib.php
login/tests/lib_test.php
mod/assign/module.js
mod/assign/styles.css
mod/forum/classes/local/exporters/discussion.php
mod/lti/locallib.php
mod/lti/tests/locallib_test.php
mod/url/locallib.php
mod/url/tests/lib_test.php
question/behaviour/interactivecountback/tests/walkthrough_test.php
question/classes/bank/edit_menu_column.php
question/engine/tests/helpers.php
question/question.php
question/tests/bank_view_test.php
question/type/match/question.php
question/type/match/questiontype.php
question/type/match/tests/backup_test.php [new file with mode: 0644]
question/type/match/tests/helper.php
question/type/match/tests/question_test.php
question/type/match/tests/questiontype_test.php
question/type/match/tests/walkthrough_test.php
version.php

index 01de61c..14a1331 100644 (file)
@@ -74,4 +74,7 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page
             new lang_string('configallowemojipickerincompatible', 'admin')
         ));
     }
+
+    $optionalsubsystems->add(new admin_setting_configcheckbox('enablemoodlenet', new lang_string('enablemoodlenet', 'admin'),
+        new lang_string('enablemoodlenet_desc', 'admin'), 1, 1, 0));
 }
diff --git a/admin/tool/dataprivacy/tests/privacy_provider_test.php b/admin/tool/dataprivacy/tests/privacy_provider_test.php
new file mode 100644 (file)
index 0000000..e671fa7
--- /dev/null
@@ -0,0 +1,187 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Tests for the plugin privacy provider
+ *
+ * @package    tool_dataprivacy
+ * @copyright  2020 Paul Holden <paulh@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+use core_privacy\local\request\userlist;
+use core_privacy\local\request\writer;
+use core_privacy\tests\provider_testcase;
+use tool_dataprivacy\api;
+use tool_dataprivacy\local\helper;
+use tool_dataprivacy\privacy\provider;
+
+/**
+ * Privacy provider tests
+ *
+ * @package    tool_dataprivacy
+ * @copyright  2020 Paul Holden <paulh@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class tool_dataprivacy_privacy_provider_testcase extends provider_testcase {
+
+    /**
+     * Test provider get_contexts_for_userid method
+     *
+     * @return void
+     */
+    public function test_get_contexts_for_userid() {
+        $this->resetAfterTest();
+
+        $user = $this->getDataGenerator()->create_user();
+        $context = context_user::instance($user->id);
+
+        // Returned context list should contain a single item.
+        $contextlist = $this->get_contexts_for_userid($user->id, 'tool_dataprivacy');
+        $this->assertCount(1, $contextlist);
+
+        // We should have the user context of our test user.
+        $this->assertSame($context, $contextlist->current());
+    }
+
+    /**
+     * Test provider get_users_in_context method
+     *
+     * @return void
+     */
+    public function test_get_users_in_context() {
+        $this->resetAfterTest();
+
+        $user = $this->getDataGenerator()->create_user();
+        $context = context_user::instance($user->id);
+
+        $userlist = new userlist($context, 'tool_dataprivacy');
+        provider::get_users_in_context($userlist);
+
+        $this->assertEquals([$user->id], $userlist->get_userids());
+    }
+
+    /**
+     * Test provider get_users_in_context method for a non-user context
+     *
+     * @return void
+     */
+    public function test_get_users_in_context_non_user_context() {
+        $context = context_system::instance();
+
+        $userlist = new userlist($context, 'tool_dataprivacy');
+        provider::get_users_in_context($userlist);
+
+        $this->assertEmpty($userlist);
+    }
+
+    /**
+     * Test provider export_user_data method
+     *
+     * @return void
+     */
+    public function test_export_user_data() {
+        $this->resetAfterTest();
+
+        $user = $this->getDataGenerator()->create_user();
+        $context = context_user::instance($user->id);
+
+        $this->setUser($user);
+
+        // Create an export request, approve it.
+        $requestexport = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT,
+            'Please export my stuff');
+        api::update_request_status($requestexport->get('id'), api::DATAREQUEST_STATUS_APPROVED);
+
+        // Create a deletion request, reject it.
+        $requestdelete = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE);
+        api::update_request_status($requestdelete->get('id'), api::DATAREQUEST_STATUS_REJECTED, 0, 'Nope');
+
+        $this->export_context_data_for_user($user->id, $context, 'tool_dataprivacy');
+
+        /** @var \core_privacy\tests\request\content_writer $writer */
+        $writer = writer::with_context($context);
+        $this->assertTrue($writer->has_any_data());
+
+        /** @var stdClass[] $data */
+        $data = (array) $writer->get_data([
+            get_string('privacyandpolicies', 'admin'),
+            get_string('datarequests', 'tool_dataprivacy'),
+        ]);
+
+        $this->assertCount(2, $data);
+
+        $strs = get_strings(['requesttypeexportshort', 'requesttypedeleteshort',
+            'statusapproved', 'statusrejected', 'creationmanual'], 'tool_dataprivacy');
+
+        // First item is the approved export request.
+        $this->assertEquals($strs->requesttypeexportshort, $data[0]->type);
+        $this->assertEquals($strs->statusapproved, $data[0]->status);
+        $this->assertEquals($strs->creationmanual, $data[0]->creationmethod);
+        $this->assertEquals($requestexport->get('comments'), $data[0]->comments);
+        $this->assertEmpty($data[0]->dpocomment);
+        $this->assertNotEmpty($data[0]->timecreated);
+
+        // Next is the rejected deletion request.
+        $this->assertEquals($strs->requesttypedeleteshort, $data[1]->type);
+        $this->assertEquals($strs->statusrejected, $data[1]->status);
+        $this->assertEquals($strs->creationmanual, $data[1]->creationmethod);
+        $this->assertEmpty($data[1]->comments);
+        $this->assertContains('Nope', $data[1]->dpocomment);
+        $this->assertNotEmpty($data[1]->timecreated);
+    }
+
+    /**
+     * Test class export_user_preferences method
+     *
+     * @return void
+     */
+    public function test_export_user_preferences() {
+        $this->resetAfterTest();
+
+        $user = $this->getDataGenerator()->create_user();
+        $this->setUser($user);
+
+        // Set filters preference.
+        $filters = [
+            helper::FILTER_TYPE . ':' . api::DATAREQUEST_TYPE_EXPORT,
+            helper::FILTER_STATUS . ':' . api::DATAREQUEST_STATUS_PENDING,
+        ];
+        set_user_preference(helper::PREF_REQUEST_FILTERS, json_encode($filters), $user);
+
+        // Set paging preference.
+        set_user_preference(helper::PREF_REQUEST_PERPAGE, 6, $user);
+
+        provider::export_user_preferences($user->id);
+
+        /** @var \core_privacy\tests\request\content_writer $writer */
+        $writer = writer::with_context(context_system::instance());
+        $this->assertTrue($writer->has_any_data());
+
+        /** @var stdClass[] $preferences */
+        $preferences = (array) $writer->get_user_preferences('tool_dataprivacy');
+        $this->assertCount(2, $preferences);
+
+        $this->assertEquals((object) [
+            'value' => '1:1, 2:0',
+            'description' => 'Type: Export, Status: Pending',
+        ], $preferences[helper::PREF_REQUEST_FILTERS]);
+
+        $this->assertEquals(6, $preferences[helper::PREF_REQUEST_PERPAGE]->value);
+    }
+}
\ No newline at end of file
index b36e515..309c478 100644 (file)
@@ -305,10 +305,17 @@ class async_helper  {
         $tabledata = array();
 
         // Get relevant backup ids based on context instance id.
-        $select = 'itemid = ? AND execution = ? AND status < ? AND status > ?';
-        $params = array($instanceid, backup::EXECUTION_DELAYED, backup::STATUS_FINISHED_ERR, backup::STATUS_NEED_PRECHECK);
-        $backups = $DB->get_records_select('backup_controllers', $select, $params, 'timecreated DESC', 'id, backupid, timecreated');
+        $select = 'itemid = :itemid AND execution = :execution AND status < :status1 AND status > :status2 ' .
+            'AND operation = :operation';
+        $params = [
+            'itemid' => $instanceid,
+            'execution' => backup::EXECUTION_DELAYED,
+            'status1' => backup::STATUS_FINISHED_ERR,
+            'status2' => backup::STATUS_NEED_PRECHECK,
+            'operation' => 'backup',
+        ];
 
+        $backups = $DB->get_records_select('backup_controllers', $select, $params, 'timecreated DESC', 'id, backupid, timecreated');
         foreach ($backups as $backup) {
             $bc = \backup_controller::load_controller($backup->backupid);  // Get the backup controller.
             $filename = $bc->get_plan()->get_setting('filename')->get_value();
index 97c4a3b..734f1c1 100644 (file)
@@ -45,5 +45,17 @@ function xmldb_filter_displayh5p_upgrade($oldversion) {
     // Automatically generated Moodle v3.8.0 release upgrade line.
     // Put any upgrade step following this.
 
+    if ($oldversion < 2020031700) {
+        // References to h5p.org has to be removed as default value for the allowedsources in the filter because H5P is going
+        // to close it down completely so that only the author can see the test content.
+        $h5porgurl = 'https://h5p.org/h5p/embed/[id]';
+        $config = get_config('filter_displayh5p', 'allowedsources');
+        if (strpos($config, $h5porgurl) !== false) {
+            set_config('allowedsources', str_replace($h5porgurl, '', $config), 'filter_displayh5p');
+        }
+
+        upgrade_plugin_savepoint(true, 2020031700, 'filter', 'displayh5p');
+    }
+
     return true;
 }
index dfb1f12..d8fc431 100644 (file)
@@ -27,6 +27,10 @@ defined('MOODLE_INTERNAL') || die;
 $string['allowedsourceslist'] = 'Allowed sources';
 $string['allowedsourceslistdesc'] = 'A list of URLs from which users can embed H5P content. If none are specified, all URLs will remain as links and not be displayed as embedded H5P content.
 
-\'[id]\' is a placeholder for the H5P content ID in the external source.';
+\'[id]\' is a placeholder for the H5P content ID in the external source.
+For example:
+
+- H5P.com: https://[xxxxxx].h5p.com/content/[id]
+- Wordpress: http://myserver/wp-admin/admin-ajax.php?action=h5p_embed&id=[id]';
 $string['filtername'] = 'Display H5P';
 $string['privacy:metadata'] = 'The display H5P filter does not store any personal data.';
index 3bed622..329155a 100644 (file)
@@ -30,5 +30,5 @@ if ($ADMIN->fulltree) {
             get_string('allowedsourceslist',
             'filter_displayh5p'),
             get_string('allowedsourceslistdesc', 'filter_displayh5p'),
-            "https://h5p.org/h5p/embed/[id]"));
+            ''));
 }
index 88a3038..e936737 100644 (file)
@@ -24,6 +24,6 @@
 
 defined('MOODLE_INTERNAL') || die;
 
-$plugin->version  = 2019111800;
+$plugin->version  = 2020031700;
 $plugin->requires = 2019111200;
 $plugin->component = 'filter_displayh5p';
index 7654b2d..7b78d06 100644 (file)
@@ -3215,21 +3215,23 @@ class H5PCore {
    * @return string
    */
   private static function hashToken($action, $time_factor) {
-    if (!isset($_SESSION['h5p_token'])) {
+    global $SESSION;
+
+    if (!isset($SESSION->h5p_token)) {
       // Create an unique key which is used to create action tokens for this session.
       if (function_exists('random_bytes')) {
-        $_SESSION['h5p_token'] = base64_encode(random_bytes(15));
+        $SESSION->h5p_token = base64_encode(random_bytes(15));
       }
       else if (function_exists('openssl_random_pseudo_bytes')) {
-        $_SESSION['h5p_token'] = base64_encode(openssl_random_pseudo_bytes(15));
+        $SESSION->h5p_token = base64_encode(openssl_random_pseudo_bytes(15));
       }
       else {
-        $_SESSION['h5p_token'] = uniqid('', TRUE);
+        $SESSION->h5p_token = uniqid('', TRUE);
       }
     }
 
     // Create hash and return
-    return substr(hash('md5', $action . $time_factor . $_SESSION['h5p_token']), -16, 13);
+    return substr(hash('md5', $action . $time_factor . $SESSION->h5p_token), -16, 13);
   }
 
   /**
index 846d9da..85463e4 100644 (file)
@@ -16,8 +16,7 @@ Added:
 
 Downloaded version: 1.24 release
 
-
-=== 3.8 ===
+Changes:
 1. In order to allow the dependency path to be overridden by child H5PCore classes, a couple of minor changes have been added to the
 h5p.classes.php file:
     - Into the getDependenciesFiles method, the line 2435:
@@ -44,6 +43,12 @@ and 1 ocurrence in h5p-metadata.class.php.
 
 3. Another PR has been sent to H5P library (https://github.com/h5p/h5p-php-library/pull/69) to fix some php74 minor problems. The same fix is being applied locally by MDL-67077. Once we import a new version, if it includes de fix, this won't be needed to reapply and can be removed.
 
+4. Replace the $_SESSION references to $SESSION. That implies that the information is saved to backends, so only the Moodle one should be used by core (core should be free from $_SESSION and always use $SESSION).
+h5p.classes.php file:
+  - Into hashToken method:
+    Declare the global $SESSION.
+    Change all the $_SESSION by $SESSION.
+A script for testing this part can be found in MDL-68068
 
 The point 2 from above won't be needed once the mbstring extension becomes mandatory in Moodle. A request has been
-sent to MDL-65809.
+sent to MDL-65809.
\ No newline at end of file
index 629745d..d4a8f67 100644 (file)
@@ -538,6 +538,8 @@ $string['enableglobalsearch_desc'] = 'If enabled, data will be indexed and synch
 $string['enablegravatar'] = 'Enable Gravatar';
 $string['enablegravatar_help'] = 'When enabled Moodle will attempt to fetch a user profile picture from Gravatar if the user has not uploaded an image.';
 $string['enablemobilewebservice'] = 'Enable web services for mobile devices';
+$string['enablemoodlenet'] = 'Enable integration with MoodleNet instances';
+$string['enablemoodlenet_desc'] = 'If enabled, and provided the MoodleNet plugin is installed, users can import content from MoodleNet into this site.';
 $string['enablerecordcache'] = 'Enable record cache';
 $string['enablerssfeeds'] = 'Enable RSS feeds';
 $string['enablesafebrowserintegration'] = 'Enable Safe Exam Browser integration';
index 1d2f972..d1604e6 100644 (file)
@@ -1027,14 +1027,25 @@ function signup_validate_data($data, $files) {
         $errors['email'] = get_string('invalidemail');
 
     } else if (empty($CFG->allowaccountssameemail)) {
-        // Make a case-insensitive query for the given email address.
-        $select = $DB->sql_equal('email', ':email', false) . ' AND mnethostid = :mnethostid';
+        // Emails in Moodle as case-insensitive and accents-sensitive. Such a combination can lead to very slow queries
+        // on some DBs such as MySQL. So we first get the list of candidate users in a subselect via more effective
+        // accent-insensitive query that can make use of the index and only then we search within that limited subset.
+        $sql = "SELECT 'x'
+                  FROM {user}
+                 WHERE " . $DB->sql_equal('email', ':email1', false, true) . "
+                   AND id IN (SELECT id
+                                FROM {user}
+                               WHERE " . $DB->sql_equal('email', ':email2', false, false) . "
+                                 AND mnethostid = :mnethostid)";
+
         $params = array(
-            'email' => $data['email'],
+            'email1' => $data['email'],
+            'email2' => $data['email'],
             'mnethostid' => $CFG->mnet_localhost_id,
         );
+
         // If there are other user(s) that already have the same email, show an error.
-        if ($DB->record_exists_select('user', $select, $params)) {
+        if ($DB->record_exists_sql($sql, $params)) {
             $forgotpasswordurl = new moodle_url('/login/forgot_password.php');
             $forgotpasswordlink = html_writer::link($forgotpasswordurl, get_string('emailexistshintlink'));
             $errors['email'] = get_string('emailexists') . ' ' . get_string('emailexistssignuphint', 'moodle', $forgotpasswordlink);
index feefb27..5bac588 100644 (file)
 $string['browserepositories'] = 'Browse repositories...';
 $string['copyrightbutton'] = 'Copyright button';
 $string['downloadbutton'] = 'Allow download';
-$string['either'] = 'Either';
 $string['embedbutton'] = 'Embed button';
-$string['enterurl'] = 'URL or embed code';
 $string['h5p:addembed'] = 'Add embedded H5P';
 $string['h5pfile'] = 'H5P file upload';
+$string['h5pfileorurl'] = 'H5P URL or file upload';
 $string['h5poptions'] = 'H5P options';
-$string['h5pproperties'] = 'H5P properties';
 $string['h5purl'] = 'H5P URL';
 $string['invalidh5purl'] = 'Invalid URL';
-$string['instructions'] = 'You can insert H5P content by <strong>either</strong> entering a URL or embed code from an external H5P site <strong>or</strong> by uploading an H5P file.';
+$string['instructions'] = 'You can insert H5P content by <strong>either</strong> entering a URL <strong>or</strong> by uploading an H5P file.';
 $string['noh5pcontent'] = 'No H5P content added';
 $string['pluginname'] = 'Insert H5P';
 $string['privacy:metadata'] = 'The atto_h5p plugin does not store any personal data.';
-$string['or'] = 'or';
\ No newline at end of file
+
+// Deprecated since Moodle 3.9.
+$string['either'] = 'Either';
+$string['enterurl'] = 'URL or embed code';
+$string['h5pproperties'] = 'H5P properties';
+$string['or'] = 'or';
diff --git a/lib/editor/atto/plugins/h5p/lang/en/deprecated.txt b/lib/editor/atto/plugins/h5p/lang/en/deprecated.txt
new file mode 100644 (file)
index 0000000..baee94f
--- /dev/null
@@ -0,0 +1,4 @@
+either,atto_h5p
+enterurl,atto_h5p
+h5pproperties,atto_h5p
+or,atto_h5p
\ No newline at end of file
index 404f2e1..5aa3457 100644 (file)
@@ -69,16 +69,13 @@ function atto_h5p_strings_for_js() {
         'copyrightbutton',
         'downloadbutton',
         'instructions',
-        'either',
         'embedbutton',
-        'enterurl',
         'h5pfile',
         'h5poptions',
-        'h5pproperties',
         'h5purl',
+        'h5pfileorurl',
         'invalidh5purl',
         'noh5pcontent',
-        'or',
         'pluginname'
     );
 
index fc51cf4..5e224f0 100644 (file)
@@ -17,7 +17,7 @@ Feature: Add h5ps to Atto
       | page     | PageName1  | PageDesc1  | 1           | C1     | H5Ptest  | 1             | 1        |
     And the "displayh5p" filter is "on"
     And the following config values are set as admin:
-      | allowedsources | https://moodle.h5p.com/content/[id]/embed | filter_displayh5p |
+      | allowedsources | https://moodle.h5p.com/content/[id] | filter_displayh5p |
 
   @javascript @external
   Scenario: Insert an embedded h5p
@@ -27,7 +27,7 @@ Feature: Add h5ps to Atto
     And I follow "PageName1"
     And I navigate to "Edit settings" in current page administration
     And I click on "Insert H5P" "button" in the "#fitem_id_page" "css_element"
-    And I set the field with xpath "//textarea[@data-region='h5purl']" to "https://moodle.h5p.com/content/1290772960722742119/embed"
+    And I set the field with xpath "//input[@data-region='h5pfile']" to "https://moodle.h5p.com/content/1290772960722742119"
     And I click on "Insert H5P" "button" in the "Insert H5P" "dialogue"
     And I wait until the page is ready
     When I click on "Save and display" "button"
@@ -64,7 +64,7 @@ Feature: Add h5ps to Atto
     And I navigate to "Edit settings" in current page administration
     And I click on "Insert H5P" "button" in the "#fitem_id_page" "css_element"
 #   This is not a real external URL, so this scenario shouldn't be labeled as external.
-    And I set the field with xpath "//textarea[@data-region='h5purl']" to "ftp://moodle.h5p.com/content/1290772960722742119/embed"
+    And I set the field with xpath "//input[@data-region='h5pfile']" to "ftp://moodle.h5p.com/content/1290772960722742119"
     When I click on "Insert H5P" "button" in the "Insert H5P" "dialogue"
     And I wait until the page is ready
     Then I should see "Invalid URL" in the "Insert H5P" "dialogue"
@@ -91,7 +91,9 @@ Feature: Add h5ps to Atto
     And I follow "PageName1"
     When I navigate to "Edit settings" in current page administration
     And I click on "Insert H5P" "button"
-    Then I should not see "URL or embed code" in the "Insert H5P" "dialogue"
+    Then I should not see "H5P URL" in the "Insert H5P" "dialogue"
+    And I should see "H5P file upload" in the "Insert H5P" "dialogue"
+    And I should see "H5P options" in the "Insert H5P" "dialogue"
 
   @javascript
   Scenario: No upload h5p capabilities
@@ -104,6 +106,8 @@ Feature: Add h5ps to Atto
     When I navigate to "Edit settings" in current page administration
     And I click on "Insert H5P" "button"
     Then I should not see "H5P file upload" in the "Insert H5P" "dialogue"
+    And I should see "H5P URL" in the "Insert H5P" "dialogue"
+    And I should not see "H5P options" in the "Insert H5P" "dialogue"
 
   @javascript @external
   Scenario: Edit H5P content
@@ -132,7 +136,7 @@ Feature: Add h5ps to Atto
     And I click on ".h5p-placeholder" "css_element"
     And I click on "Insert H5P" "button" in the "#fitem_id_page" "css_element"
 #   External URL
-    And I set the field with xpath "//textarea[@data-region='h5purl']" to "https://moodle.h5p.com/content/1290772960722742119/embed"
+    And I set the field with xpath "//input[@data-region='h5pfile']" to "https://moodle.h5p.com/content/1290772960722742119"
     And I click on "Insert H5P" "button" in the "Insert H5P" "dialogue"
     And I wait until the page is ready
     And I click on "Save and display" "button"
@@ -199,6 +203,32 @@ Feature: Add h5ps to Atto
     And I should see "Embed"
     And I should see "Rights of use"
 
+  @javascript @external
+  Scenario: H5P options are ignored for H5P URLs
+    Given I log in as "admin"
+    And I change window size to "large"
+    And I am on "Course 1" course homepage
+    And I follow "PageName1"
+    And I navigate to "Edit settings" in current page administration
+    And I click on "Insert H5P" "button" in the "#fitem_id_page" "css_element"
+    And I set the field with xpath "//input[@data-region='h5pfile']" to "https://moodle.h5p.com/content/1290752078589054689"
+    And I click on "H5P options" "link"
+    And I click on "Embed button" "checkbox"
+    And I click on "Insert H5P" "button" in the "Insert H5P" "dialogue"
+    And I wait until the page is ready
+    When I click on "Save and display" "button"
+    Then ".h5p-placeholder" "css_element" should exist
+    And I wait until the page is ready
+    And I switch to "h5pcontent" iframe
+    And I should see "History of strawberries"
+    And I should not see "Embed"
+    And I switch to the main frame
+    And I navigate to "Edit settings" in current page administration
+    And I click on ".h5p-placeholder" "css_element"
+    And I click on "Insert H5P" "button" in the "#fitem_id_page" "css_element"
+    And I click on "H5P options" "link"
+    And "input[aria-label=\"Embed button\"]:not([checked=checked])" "css_element" should exist
+
   @javascript
   Scenario: Private H5P files are shown to students
     Given the following "users" exist:
index 29ed69b..98d1d42 100644 (file)
Binary files a/lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button-debug.js and b/lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button-debug.js differ
index 41c5f75..7fe48a7 100644 (file)
Binary files a/lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button-min.js and b/lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button-min.js differ
index 29ed69b..98d1d42 100644 (file)
Binary files a/lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button.js and b/lib/editor/atto/plugins/h5p/yui/build/moodle-atto_h5p-button/moodle-atto_h5p-button.js differ
index 4b753ba..2906734 100644 (file)
@@ -36,7 +36,6 @@ var CSS = {
         H5PBROWSER: 'openh5pbrowser',
         INPUTALT: 'atto_h5p_altentry',
         INPUTH5PFILE: 'atto_h5p_file',
-        INPUTH5PURL: 'atto_h5p_url',
         INPUTSUBMIT: 'atto_h5p_urlentrysubmit',
         OPTION_DOWNLOAD_BUTTON: 'atto_h5p_option_download_button',
         OPTION_COPYRIGHT_BUTTON: 'atto_h5p_option_copyright_button',
@@ -47,7 +46,6 @@ var CSS = {
         CONTENTWARNING: '.' + CSS.CONTENTWARNING,
         H5PBROWSER: '.' + CSS.H5PBROWSER,
         INPUTH5PFILE: '.' + CSS.INPUTH5PFILE,
-        INPUTH5PURL: '.' + CSS.INPUTH5PURL,
         INPUTSUBMIT: '.' + CSS.INPUTSUBMIT,
         OPTION_DOWNLOAD_BUTTON: '.' + CSS.OPTION_DOWNLOAD_BUTTON,
         OPTION_COPYRIGHT_BUTTON: '.' + CSS.OPTION_COPYRIGHT_BUTTON,
@@ -62,65 +60,71 @@ var CSS = {
                 '<div style="display:none" role="alert" class="alert alert-warning mb-1 {{CSS.CONTENTWARNING}}">' +
                     '{{get_string "noh5pcontent" component}}' +
                 '</div>' +
-                '{{#if canUploadAndEmbed}}' +
-                    '<div class="mt-2 attoh5pinstructions">{{{get_string "instructions" component}}}</div>' +
-                    '<div class="my-2"><strong>{{get_string "either" component}}</strong></div>' +
-                '{{/if}}' +
-                '{{#if canEmbed}}' +
-                '<div class="mb-4">' +
-                    '<label for="{{elementid}}_{{CSS.INPUTH5PURL}}">{{get_string "enterurl" component}}</label>' +
-                    '<div style="display:none" role="alert" class="alert alert-warning mb-1 {{CSS.URLWARNING}}">' +
-                        '{{get_string "invalidh5purl" component}}' +
-                    '</div>' +
-                    '<textarea rows="3" data-region="h5purl" class="form-control {{CSS.INPUTH5PURL}}" type="url" ' +
-                    'id="{{elementid}}_{{CSS.INPUTH5PURL}}" />{{embedURL}}</textarea>' +
+                '<div style="display:none" role="alert" class="alert alert-warning mb-1 {{CSS.URLWARNING}}">' +
+                    '{{get_string "invalidh5purl" component}}' +
                 '</div>' +
-                '{{/if}}' +
                 '{{#if canUploadAndEmbed}}' +
-                    '<div class="my-2"><strong>{{get_string "or" component}}</strong></div>' +
+                    '<div class="mt-2 mb-4 attoh5pinstructions">{{{get_string "instructions" component}}}</div>' +
                 '{{/if}}' +
-                '{{#if canUpload}}' +
                 '<div class="mb-4">' +
-                    '<label for="{{elementid}}_{{CSS.H5PBROWSER}}">{{get_string "h5pfile" component}}</label>' +
+                    '<label for="{{elementid}}_{{CSS.H5PBROWSER}}">' +
+                        '{{#if canUploadAndEmbed}}' +
+                            '{{get_string "h5pfileorurl" component}}' +
+                        '{{/if}}' +
+                        '{{^if canUploadAndEmbed}}' +
+                            '{{#if canUpload}}' +
+                                '{{get_string "h5pfile" component}}' +
+                            '{{/if}}' +
+                            '{{#if canEmbed}}' +
+                                '{{get_string "h5purl" component}}' +
+                            '{{/if}}' +
+                        '{{/if}}' +
+                    '</label>' +
                     '<div class="input-group input-append w-100">' +
                         '<input class="form-control {{CSS.INPUTH5PFILE}}" type="url" value="{{fileURL}}" ' +
-                        'id="{{elementid}}_{{CSS.INPUTH5PFILE}}" size="32"/>' +
-                        '<span class="input-group-append">' +
-                            '<button class="btn btn-secondary {{CSS.H5PBROWSER}}" type="button">' +
-                            '{{get_string "browserepositories" component}}</button>' +
-                        '</span>' +
+                        'id="{{elementid}}_{{CSS.INPUTH5PFILE}}" data-region="h5pfile" size="32"/>' +
+                        '{{#if canUpload}}' +
+                            '<span class="input-group-append">' +
+                                '<button class="btn btn-secondary {{CSS.H5PBROWSER}}" type="button">' +
+                                '{{get_string "browserepositories" component}}</button>' +
+                            '</span>' +
+                        '{{/if}}' +
                     '</div>' +
-                    '<fieldset class="collapsible {{#if collapseOptions}}collapsed{{/if}}" id="{{elementid}}_h5poptions">' +
-                        '<legend class="ftoggler">{{get_string "h5poptions" component}}</legend>' +
-                        '<div class="fcontainer">' +
-                            '<div class="form-check">' +
-                                '<input type="checkbox" {{optionDownloadButton}} ' +
-                                'class="form-check-input {{CSS.OPTION_DOWNLOAD_BUTTON}}"' +
-                                'id="{{elementid}}_h5p-option-allow-download"/>' +
-                                '<label class="form-check-label" for="{{elementid}}_h5p-option-allow-download">' +
-                                '{{get_string "downloadbutton" component}}' +
-                                '</label>' +
-                            '</div>' +
-                            '<div class="form-check">' +
-                                '<input type="checkbox" {{optionEmbedButton}} ' +
-                                'class="form-check-input {{CSS.OPTION_EMBED_BUTTON}}" ' +
-                                    'id="{{elementid}}_h5p-option-embed-button"/>' +
-                                '<label class="form-check-label" for="{{elementid}}_h5p-option-embed-button">' +
-                                '{{get_string "embedbutton" component}}' +
-                                '</label>' +
-                            '</div>' +
-                            '<div class="form-check mb-2">' +
-                                '<input type="checkbox" {{optionCopyrightButton}} ' +
-                                'class="form-check-input {{CSS.OPTION_COPYRIGHT_BUTTON}}" ' +
-                                    'id="{{elementid}}_h5p-option-copyright-button"/>' +
-                                '<label class="form-check-label" for="{{elementid}}_h5p-option-copyright-button">' +
-                                '{{get_string "copyrightbutton" component}}' +
-                                '</label>' +
+                    '{{#if canUpload}}' +
+                        '<fieldset class="collapsible {{#if collapseOptions}}collapsed{{/if}}" id="{{elementid}}_h5poptions">' +
+                            '<legend class="ftoggler">{{get_string "h5poptions" component}}</legend>' +
+                            '<div class="fcontainer">' +
+                                '<div class="form-check">' +
+                                    '<input type="checkbox" {{optionDownloadButton}} ' +
+                                    'class="form-check-input {{CSS.OPTION_DOWNLOAD_BUTTON}}"' +
+                                    'aria-label="{{get_string "downloadbutton" component}}" ' +
+                                    'id="{{elementid}}_h5p-option-allow-download"/>' +
+                                    '<label class="form-check-label" for="{{elementid}}_h5p-option-allow-download">' +
+                                    '{{get_string "downloadbutton" component}}' +
+                                    '</label>' +
+                                '</div>' +
+                                '<div class="form-check">' +
+                                    '<input type="checkbox" {{optionEmbedButton}} ' +
+                                    'class="form-check-input {{CSS.OPTION_EMBED_BUTTON}}" ' +
+                                    'aria-label="{{get_string "embedbutton" component}}" ' +
+                                        'id="{{elementid}}_h5p-option-embed-button"/>' +
+                                    '<label class="form-check-label" for="{{elementid}}_h5p-option-embed-button">' +
+                                    '{{get_string "embedbutton" component}}' +
+                                    '</label>' +
+                                '</div>' +
+                                '<div class="form-check mb-2">' +
+                                    '<input type="checkbox" {{optionCopyrightButton}} ' +
+                                    'class="form-check-input {{CSS.OPTION_COPYRIGHT_BUTTON}}" ' +
+                                    'aria-label="{{get_string "copyrightbutton" component}}" ' +
+                                        'id="{{elementid}}_h5p-option-copyright-button"/>' +
+                                    '<label class="form-check-label" for="{{elementid}}_h5p-option-copyright-button">' +
+                                    '{{get_string "copyrightbutton" component}}' +
+                                    '</label>' +
+                                '</div>' +
                             '</div>' +
-                        '</div>' +
-                    '</fieldset>' +
+                        '</fieldset>' +
+                    '{{/if}}' +
                 '</div>' +
-                '{{/if}}' +
                 '<div class="text-center">' +
                 '<button class="btn btn-secondary {{CSS.INPUTSUBMIT}}" type="submit">' + '' +
                     '{{get_string "pluginname" component}}</button>' +
@@ -268,9 +272,9 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
      */
     _getPermissions: function() {
         var permissions = {
+            'canEmbed': false,
             'canUpload': false,
-            'canUploadAndEbmed': false,
-            'canEmbed': false
+            'canUploadAndEmbed': false
         };
 
         if (this.get('host').canShowFilepicker('h5p')) {
@@ -302,7 +306,6 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
         var permissions = this._getPermissions();
 
         var fileURL,
-            embedURL,
             optionDownloadButton,
             optionEmbedButton,
             optionCopyrightButton,
@@ -332,7 +335,7 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
                     }
                 }
             } else {
-                embedURL = H5PURL;
+                fileURL = H5PURL;
             }
         }
 
@@ -343,10 +346,9 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
                 component: COMPONENTNAME,
                 canUpload: permissions.canUpload,
                 canEmbed: permissions.canEmbed,
-                fileURL: fileURL,
-                embedURL: embedURL,
                 canUploadAndEmbed: permissions.canUploadAndEmbed,
                 collapseOptions: collapseOptions,
+                fileURL: fileURL,
                 optionDownloadButton: optionDownloadButton,
                 optionEmbedButton: optionEmbedButton,
                 optionCopyrightButton: optionCopyrightButton
@@ -372,7 +374,6 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
         if (params.url !== '') {
             var input = this._form.one(SELECTORS.INPUTH5PFILE);
             input.set('value', params.url);
-            this._form.one(SELECTORS.INPUTH5PURL).set('value', '');
             this._removeWarnings();
         }
     },
@@ -397,11 +398,6 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
 
         if (permissions.canUploadAndEmbed) {
             form.one(SELECTORS.INPUTH5PFILE).on('change', function() {
-                form.one(SELECTORS.INPUTH5PURL).set('value', '');
-                this._removeWarnings();
-            }, this);
-            form.one(SELECTORS.INPUTH5PURL).on('change', function() {
-                form.one(SELECTORS.INPUTH5PFILE).set('value', '');
                 this._removeWarnings();
             }, this);
         }
@@ -421,7 +417,6 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
 
     /**
      * Update the h5p in the contenteditable.
-
      *
      * @method _setH5P
      * @param {EventFacade} e
@@ -429,20 +424,11 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
      */
     _setH5P: function(e) {
         var form = this._form,
-            url = form.one(SELECTORS.INPUTH5PURL).get('value'),
             h5phtml,
             host = this.get('host'),
-            h5pfile,
+            h5pfile = form.one(SELECTORS.INPUTH5PFILE).get('value'),
             permissions = this._getPermissions();
 
-        if (permissions.canEmbed) {
-            url = form.one(SELECTORS.INPUTH5PURL).get('value');
-        }
-
-        if (permissions.canUpload) {
-            h5pfile = form.one(SELECTORS.INPUTH5PFILE).get('value');
-        }
-
         e.preventDefault();
 
         // Check if there are any issues.
@@ -462,68 +448,49 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
             addParagraphs = false;
         }
 
-        if (url !== '') {
-
+        if (h5pfile !== '') {
             host.setSelection(this._currentSelection);
 
-            if (this._validEmbed(url)) {
-                var embedtemplate = Y.Handlebars.compile(H5PTEMPLATE);
-                var regex = /<iframe.*?src="(.*?)".*<\/iframe>/;
-                var src = url.match(regex)[1];
+            if (h5pfile.startsWith(M.cfg.wwwroot)) {
+                // It's a local file.
+                var params = '';
+                if (permissions.canUpload) {
+                    var options = {};
+                    if (form.one(SELECTORS.OPTION_DOWNLOAD_BUTTON).get('checked')) {
+                        options['export'] = '1';
+                    }
+                    if (form.one(SELECTORS.OPTION_EMBED_BUTTON).get('checked')) {
+                        options.embed = '1';
+                    }
+                    if (form.one(SELECTORS.OPTION_COPYRIGHT_BUTTON).get('checked')) {
+                        options.copyright = '1';
+                    }
 
-                // In case a local H5P embed code is used we need get the url
-                // param form the src and decode it.
-                if (src.startsWith(M.cfg.wwwroot + '/h5p/embed.php')) {
-                    src = decodeURIComponent(src.split("url=")[1]);
+                    for (var opt in options) {
+                        if (params === "" && (h5pfile.indexOf("?") === -1)) {
+                            params += "?";
+                        } else {
+                            params += "&amp;";
+                        }
+                        params += opt + "=" + options[opt];
+                    }
                 }
 
-                h5phtml = embedtemplate({
-                    url: src
+                var h5ptemplate = Y.Handlebars.compile(H5PTEMPLATE);
+
+                h5phtml = h5ptemplate({
+                    url: h5pfile + params,
+                    addParagraphs: addParagraphs
                 });
             } else {
+                // It's a URL.
                 var urltemplate = Y.Handlebars.compile(H5PTEMPLATE);
                 h5phtml = urltemplate({
-                    url: url
+                    url: h5pfile
                 });
             }
 
-            this.get('host').insertContentAtFocusPoint(h5phtml);
-
-            this.markUpdated();
-        } else if (h5pfile !== '') {
-
-            host.setSelection(this._currentSelection);
-
-            var options = {};
-
-            if (form.one(SELECTORS.OPTION_DOWNLOAD_BUTTON).get('checked')) {
-                options['export'] = '1';
-            }
-            if (form.one(SELECTORS.OPTION_EMBED_BUTTON).get('checked')) {
-                options.embed = '1';
-            }
-            if (form.one(SELECTORS.OPTION_COPYRIGHT_BUTTON).get('checked')) {
-                options.copyright = '1';
-            }
-
-            var params = "";
-            for (var opt in options) {
-                if (params === "" && (h5pfile.indexOf("?") === -1)) {
-                    params += "?";
-                } else {
-                    params += "&amp;";
-                }
-                params += opt + "=" + options[opt];
-            }
-
-            var h5ptemplate = Y.Handlebars.compile(H5PTEMPLATE);
-
-            h5phtml = h5ptemplate({
-                url: h5pfile + params,
-                addParagraphs: addParagraphs
-            });
-
-            this.get('host').insertContentAtFocusPoint(h5phtml);
+            host.insertContentAtFocusPoint(h5phtml);
 
             this.markUpdated();
         }
@@ -572,30 +539,21 @@ Y.namespace('M.atto_h5p').Button = Y.Base.create('button', Y.M.editor_atto.Edito
     _updateWarning: function() {
         var form = this._form,
             state = true,
-            url,
             h5pfile,
             permissions = this._getPermissions();
 
-
-        if (permissions.canEmbed) {
-            url = form.one(SELECTORS.INPUTH5PURL).get('value');
-            if (url !== '') {
-                if (this._validURL(url) || this._validEmbed(url)) {
+        if (permissions.canUpload || permissions.canEmbed) {
+            h5pfile = form.one(SELECTORS.INPUTH5PFILE).get('value');
+            if (h5pfile !== '') {
+                form.one(SELECTORS.CONTENTWARNING).setStyle('display', 'none');
+                if (h5pfile.startsWith(M.cfg.wwwroot) || this._validURL(h5pfile)) {
+                    // Only external URLs have to be validated.
                     form.one(SELECTORS.URLWARNING).setStyle('display', 'none');
                     state = false;
                 } else {
                     form.one(SELECTORS.URLWARNING).setStyle('display', 'block');
                     state = true;
                 }
-                return state;
-            }
-        }
-
-        if (permissions.canUpload) {
-            h5pfile = form.one(SELECTORS.INPUTH5PFILE).get('value');
-            if (h5pfile !== '') {
-                form.one(SELECTORS.CONTENTWARNING).setStyle('display', 'none');
-                state = false;
             } else {
                 form.one(SELECTORS.CONTENTWARNING).setStyle('display', 'block');
                 state = true;
index e879546..5441dbd 100644 (file)
@@ -4913,11 +4913,15 @@ function get_complete_user_data($field, $value, $mnethostid = null, $throwexcept
     // Build the WHERE clause for an SQL query.
     $params = array('fieldval' => $value);
 
-    // Do a case-insensitive query, if necessary.
+    // Do a case-insensitive query, if necessary. These are generally very expensive. The performance can be improved on some DBs
+    // such as MySQL by pre-filtering users with accent-insensitive subselect.
     if (in_array($field, $caseinsensitivefields)) {
         $fieldselect = $DB->sql_equal($field, ':fieldval', false);
+        $idsubselect = $DB->sql_equal($field, ':fieldval2', false, false);
+        $params['fieldval2'] = $value;
     } else {
         $fieldselect = "$field = :fieldval";
+        $idsubselect = '';
     }
     $constraints = "$fieldselect AND deleted <> 1";
 
@@ -4934,6 +4938,10 @@ function get_complete_user_data($field, $value, $mnethostid = null, $throwexcept
         $constraints .= " AND mnethostid = :mnethostid";
     }
 
+    if ($idsubselect) {
+        $constraints .= " AND id IN (SELECT id FROM {user} WHERE {$idsubselect})";
+    }
+
     // Get all the basic user data.
     try {
         // Make sure that there's only a single record that matches our query.
index 56a7e38..2a894ee 100644 (file)
@@ -2525,7 +2525,6 @@ class core_renderer extends renderer_base {
 
         $attributes = array('src' => $src, 'class' => $class, 'width' => $size, 'height' => $size);
         if (!$userpicture->visibletoscreenreaders) {
-            $attributes['role'] = 'presentation';
             $alt = '';
             $attributes['aria-hidden'] = 'true';
         }
index 1447623..7e17307 100644 (file)
@@ -1609,7 +1609,8 @@ class moodle_page {
         }
 
         $mnetpeertheme = '';
-        if (isloggedin() and isset($CFG->mnet_localhost_id) and $USER->mnethostid != $CFG->mnet_localhost_id) {
+        $mnetvarsok = isset($CFG->mnet_localhost_id) && isset($USER->mnethostid);
+        if (isloggedin() and $mnetvarsok and $USER->mnethostid != $CFG->mnet_localhost_id) {
             require_once($CFG->dirroot.'/mnet/peer.php');
             $mnetpeer = new mnet_peer();
             $mnetpeer->set_id($USER->mnethostid);
index 164e21d..4b83105 100644 (file)
@@ -2383,3 +2383,38 @@ function question_module_uses_questions($modname) {
 
     return false;
 }
+
+/**
+ * If $oldidnumber ends in some digits then return the next available idnumber of the same form.
+ *
+ * So idnum -> null (no digits at the end) idnum0099 -> idnum0100 (if that is unused,
+ * else whichever of idnum0101, idnume0102, ... is unused. idnum9 -> idnum10.
+ *
+ * @param string $oldidnumber a question idnumber.
+ * @param int $categoryid a question category id.
+ * @return string|null suggested new idnumber for a question in that category, or null if one cannot be found.
+ */
+function core_question_find_next_unused_idnumber(string $oldidnumber, int $categoryid):? string {
+    global $DB;
+
+    // The the old idnumber is not of the right form, bail now.
+    if (!preg_match('~\d+$~', $oldidnumber, $matches)) {
+        return null;
+    }
+
+    // Find all used idnumbers in one DB query.
+    $usedidnumbers = $DB->get_records_select_menu('question', 'category = ? AND idnumber IS NOT NULL',
+            [$categoryid], '', 'idnumber, 1');
+
+    // Find the next unused idnumber.
+    $newidnumber = $oldidnumber;
+    do {
+        // If we have got to something9999, insert an extra digit before incrementing.
+        if (preg_match('~^(.*[^0-9])(9+)$~', $newidnumber, $matches)) {
+            $newidnumber = $matches[1] . '0' . $matches[2];
+        }
+        $newidnumber++;
+    } while (isset($usedidnumbers[$newidnumber]));
+
+    return (string) $newidnumber;
+}
index f0a3d67..99f5923 100644 (file)
@@ -414,4 +414,62 @@ class core_authlib_testcase extends advanced_testcase {
             $this->assertInstanceOf('coding_exception', $e);
         }
     }
+
+    /**
+     * Test the {@link signup_validate_data()} duplicate email validation.
+     */
+    public function test_signup_validate_data_same_email() {
+        global $CFG;
+        require_once($CFG->libdir . '/authlib.php');
+        require_once($CFG->dirroot . '/user/profile/lib.php');
+
+        $this->resetAfterTest();
+
+        $CFG->registerauth = 'email';
+        $CFG->passwordpolicy = false;
+
+        // In this test, we want to check accent-sensitive email search. However, accented email addresses do not pass
+        // the default `validate_email()` and Moodle does not yet provide a CFG switch to allow such emails.  So we
+        // inject our own validation method here and revert it back once we are done. This custom validator method is
+        // identical to the default 'php' validator with the only difference: it has the FILTER_FLAG_EMAIL_UNICODE set
+        // so that it allows to use non-ASCII characters in email addresses.
+        $defaultvalidator = moodle_phpmailer::$validator; moodle_phpmailer::$validator = function($address) {
+            return (bool) filter_var($address, FILTER_VALIDATE_EMAIL, FILTER_FLAG_EMAIL_UNICODE);
+        };
+
+        // Check that two users cannot share the same email address if the site is configured so.
+        // Emails in Moodle are supposed to be case-insensitive (and accent-sensitive but accents are not yet supported).
+        $CFG->allowaccountssameemail = false;
+
+        $u1 = $this->getDataGenerator()->create_user([
+            'username' => 'abcdef',
+            'email' => 'abcdef@example.com',
+        ]);
+
+        $formdata = [
+            'username' => 'newuser',
+            'firstname' => 'First',
+            'lastname' => 'Last',
+            'password' => 'weak',
+            'email' => 'ABCDEF@example.com',
+        ];
+
+        $errors = signup_validate_data($formdata, []);
+        $this->assertContains('This email address is already registered.', $errors['email']);
+
+        // Emails are accent-sensitive though so if we change a -> á in the u1's email, it should pass.
+        // Please note that Moodle does not normally support such emails yet. We test the DB search sensitivity here.
+        $formdata['email'] = 'ábcdef@example.com';
+        $errors = signup_validate_data($formdata, []);
+        $this->assertArrayNotHasKey('email', $errors);
+
+        // Check that users can share the same email if the site is configured so.
+        $CFG->allowaccountssameemail = true;
+        $formdata['email'] = 'abcdef@example.com';
+        $errors = signup_validate_data($formdata, []);
+        $this->assertArrayNotHasKey('email', $errors);
+
+        // Restore the original email address validator.
+        moodle_phpmailer::$validator = $defaultvalidator;
+    }
 }
index cd04ace..3142b26 100644 (file)
@@ -2141,4 +2141,47 @@ class core_questionlib_testcase extends advanced_testcase {
                 ['id' => $systemq->id, 'courseid' => SITEID, 'sesskey' => sesskey()]),
                 question_get_export_single_question_url(question_bank::load_question($systemq->id)));
     }
+
+    /**
+     * Get test cases for test_core_question_find_next_unused_idnumber.
+     *
+     * @return array test cases.
+     */
+    public function find_next_unused_idnumber_cases(): array {
+        return [
+            ['id', null],
+            ['id1a', null],
+            ['id001', 'id002'],
+            ['id9', 'id10'],
+            ['id009', 'id010'],
+            ['id999', 'id1000'],
+        ];
+    }
+
+    /**
+     * Test core_question_find_next_unused_idnumber in the case when there are no other questions.
+     *
+     * @dataProvider find_next_unused_idnumber_cases
+     * @param string $oldidnumber value to pass to core_question_find_next_unused_idnumber.
+     * @param string|null $expectednewidnumber expected result.
+     */
+    public function test_core_question_find_next_unused_idnumber(string $oldidnumber, ?string $expectednewidnumber) {
+        $this->assertSame($expectednewidnumber, core_question_find_next_unused_idnumber($oldidnumber, 0));
+    }
+
+    public function test_core_question_find_next_unused_idnumber_skips_used() {
+        $this->resetAfterTest();
+
+        /** @var core_question_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_question');
+        $category = $generator->create_question_category();
+        $othercategory = $generator->create_question_category();
+        $generator->create_question('truefalse', null, ['category' => $category->id, 'idnumber' => 'id9']);
+        $generator->create_question('truefalse', null, ['category' => $category->id, 'idnumber' => 'id10']);
+        // Next one to make sure only idnumbers from the right category are ruled out.
+        $generator->create_question('truefalse', null, ['category' => $othercategory->id, 'idnumber' => 'id11']);
+
+        $this->assertSame('id11', core_question_find_next_unused_idnumber('id9', $category->id));
+        $this->assertSame('id11', core_question_find_next_unused_idnumber('id8', $category->id));
+    }
 }
index b391ea7..e064405 100644 (file)
@@ -24,6 +24,9 @@
  * @copyright  Peter Bulmer
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
+
+defined('MOODLE_INTERNAL') || die();
+
 define('PWRESET_STATUS_NOEMAILSENT', 1);
 define('PWRESET_STATUS_TOKENSENT', 2);
 define('PWRESET_STATUS_OTHEREMAILSENT', 3);
@@ -93,14 +96,31 @@ function core_login_process_password_reset($username, $email) {
         $user = $DB->get_record('user', $userparams);
     } else {
         // Try to load the user record based on email address.
-        // this is tricky because
+        // This is tricky because:
         // 1/ the email is not guaranteed to be unique - TODO: send email with all usernames to select the account for pw reset
         // 2/ mailbox may be case sensitive, the email domain is case insensitive - let's pretend it is all case-insensitive.
-
-        $select = $DB->sql_like('email', ':email', false, true, false, '|') .
-                " AND mnethostid = :mnethostid AND deleted=0 AND suspended=0";
-        $params = array('email' => $DB->sql_like_escape($email, '|'), 'mnethostid' => $CFG->mnet_localhost_id);
-        $user = $DB->get_record_select('user', $select, $params, '*', IGNORE_MULTIPLE);
+        //
+        // The case-insensitive + accent-sensitive search may be expensive as some DBs such as MySQL cannot use the
+        // index in that case. For that reason, we first perform accent-insensitive search in a subselect for potential
+        // candidates (which can use the index) and only then perform the additional accent-sensitive search on this
+        // limited set of records in the outer select.
+        $sql = "SELECT *
+                  FROM {user}
+                 WHERE " . $DB->sql_equal('email', ':email1', false, true) . "
+                   AND id IN (SELECT id
+                                FROM {user}
+                               WHERE mnethostid = :mnethostid
+                                 AND deleted = 0
+                                 AND suspended = 0
+                                 AND " . $DB->sql_equal('email', ':email2', false, false) . ")";
+
+        $params = array(
+            'email1' => $email,
+            'email2' => $email,
+            'mnethostid' => $CFG->mnet_localhost_id,
+        );
+
+        $user = $DB->get_record_sql($sql, $params, IGNORE_MULTIPLE);
     }
 
     // Target user details have now been identified, or we know that there is no such account.
index a32941c..02bc226 100644 (file)
@@ -355,4 +355,75 @@ class core_login_lib_testcase extends advanced_testcase {
             $this->assertArrayNotHasKey('email', $validationerrors);
         }
     }
+
+    /**
+     * Test searching for the user record by matching the provided email address when resetting password.
+     *
+     * Email addresses should be handled as case-insensitive but accent sensitive.
+     */
+    public function test_core_login_process_password_reset_email_sensitivity() {
+        global $CFG;
+        require_once($CFG->libdir.'/phpmailer/moodle_phpmailer.php');
+
+        $this->resetAfterTest();
+        $sink = $this->redirectEmails();
+        $CFG->protectusernames = 0;
+
+        // In this test, we need to mock sending emails on non-ASCII email addresses. However, such email addresses do
+        // not pass the default `validate_email()` and Moodle does not yet provide a CFG switch to allow such emails.
+        // So we inject our own validation method here and revert it back once we are done. This custom validator method
+        // is identical to the default 'php' validator with the only difference: it has the FILTER_FLAG_EMAIL_UNICODE
+        // set so that it allows to use non-ASCII characters in email addresses.
+        $defaultvalidator = moodle_phpmailer::$validator;
+        moodle_phpmailer::$validator = function($address) {
+            return (bool) filter_var($address, FILTER_VALIDATE_EMAIL, FILTER_FLAG_EMAIL_UNICODE);
+        };
+
+        // Emails are treated as case-insensitive when searching for the matching user account.
+        $u1 = $this->getDataGenerator()->create_user(['email' => 'priliszlutouckykunupeldabelskeody@example.com']);
+
+        list($status, $notice, $url) = core_login_process_password_reset(null, 'PrIlIsZlUtOuCkYKuNupELdAbElSkEoDy@eXaMpLe.CoM');
+
+        $this->assertSame('emailresetconfirmsent', $status);
+        $emails = $sink->get_messages();
+        $this->assertCount(1, $emails);
+        $email = reset($emails);
+        $this->assertSame($u1->email, $email->to);
+        $sink->clear();
+
+        // There may exist two users with same emails.
+        $u2 = $this->getDataGenerator()->create_user(['email' => 'PRILISZLUTOUCKYKUNUPELDABELSKEODY@example.com']);
+
+        list($status, $notice, $url) = core_login_process_password_reset(null, 'PrIlIsZlUtOuCkYKuNupELdAbElSkEoDy@eXaMpLe.CoM');
+
+        $this->assertSame('emailresetconfirmsent', $status);
+        $emails = $sink->get_messages();
+        $this->assertCount(1, $emails);
+        $email = reset($emails);
+        $this->assertSame(core_text::strtolower($u2->email), core_text::strtolower($email->to));
+        $sink->clear();
+
+        // However, emails are accent sensitive - note this is the u1's email with a single character a -> á changed.
+        list($status, $notice, $url) = core_login_process_password_reset(null, 'priliszlutouckykunupeldábelskeody@example.com');
+
+        $this->assertSame('emailpasswordconfirmnotsent', $status);
+        $emails = $sink->get_messages();
+        $this->assertCount(0, $emails);
+        $sink->clear();
+
+        $u3 = $this->getDataGenerator()->create_user(['email' => 'PřílišŽluťoučkýKůňÚpělĎálebskéÓdy@example.com']);
+
+        list($status, $notice, $url) = core_login_process_password_reset(null, 'pŘÍLIŠžLuŤOuČkÝkŮŇúPĚLďÁLEBSKÉóDY@eXaMpLe.CoM');
+
+        $this->assertSame('emailresetconfirmsent', $status);
+        $emails = $sink->get_messages();
+        $this->assertCount(1, $emails);
+        $email = reset($emails);
+        $this->assertSame($u3->email, $email->to);
+        $sink->clear();
+
+        // Restore the original email address validator.
+        moodle_phpmailer::$validator = $defaultvalidator;
+    }
+
 }
index 70a1d60..111003b 100644 (file)
@@ -169,7 +169,7 @@ M.mod_assign.init_plugin_summary = function(Y, subtype, type, submissionid) {
     if (contract) {
         contract.on('click', function(e) {
             e.preventDefault();
-            var link = e.target;
+            var link = e.currentTarget || e.target;
             var linkclasses = link.getAttribute('class').split(' ');
             var thissuffix = '';
             for (var i = 0; i < linkclasses.length; i++) {
@@ -202,7 +202,7 @@ M.mod_assign.init_plugin_summary = function(Y, subtype, type, submissionid) {
     if (expand) {
         expand.on('click', function(e) {
             e.preventDefault();
-            var link = e.target;
+            var link = e.currentTarget || e.target;
             var linkclasses = link.getAttribute('class').split(' ');
             var thissuffix = '';
             for (var i = 0; i < linkclasses.length; i++) {
index f3cc290..f936282 100644 (file)
     display: none;
 }
 
-.path-mod-assign .expandsummaryicon i {
-    pointer-events: none;
-}
-
 .path-mod-assign.jsenabled .expandsummaryicon {
     display: inline-block;
 }
index 44e11b0..ede9dc1 100644 (file)
@@ -178,13 +178,13 @@ class discussion extends exporter {
                     'urls' => [],
                 ];
 
-                if (!$group->hidepicture) {
-                    $url = get_group_picture_url($group, $forum->get_course_id(), true);
-                    if (empty($url)) {
-                        // Get a generic group image URL.
-                        $url = $output->image_url('g/g1')->out(false);
-                    }
+                // If not hiding the group picture, and the group has a picture then use it. Fallback to generic group image.
+                if (!$group->hidepicture &&
+                        ($url = get_group_picture_url($group, $forum->get_course_id(), true))) {
+
                     $groupdata['urls']['picture'] = $url;
+                } else {
+                    $groupdata['urls']['picture'] = $output->image_url('g/g1')->out(false);
                 }
 
                 if ($capabilitymanager->can_view_participants($user, $discussion)) {
index d8e1ef8..4cd7786 100644 (file)
@@ -3278,7 +3278,44 @@ function lti_post_launch_html($newparms, $endpoint, $debug=false) {
  */
 function lti_initiate_login($courseid, $id, $instance, $config, $messagetype = 'basic-lti-launch-request', $title = '',
         $text = '') {
-    global $SESSION, $USER, $CFG;
+    global $SESSION;
+
+    $params = lti_build_login_request($courseid, $id, $instance, $config, $messagetype);
+    $SESSION->lti_message_hint = "{$courseid},{$config->typeid},{$id}," . base64_encode($title) . ',' .
+        base64_encode($text);
+
+    $r = "<form action=\"" . $config->lti_initiatelogin .
+        "\" name=\"ltiInitiateLoginForm\" id=\"ltiInitiateLoginForm\" method=\"post\" " .
+        "encType=\"application/x-www-form-urlencoded\">\n";
+
+    foreach ($params as $key => $value) {
+        $key = htmlspecialchars($key);
+        $value = htmlspecialchars($value);
+        $r .= "  <input type=\"hidden\" name=\"{$key}\" value=\"{$value}\"/>\n";
+    }
+    $r .= "</form>\n";
+
+    $r .= "<script type=\"text/javascript\">\n" .
+        "//<![CDATA[\n" .
+        "document.ltiInitiateLoginForm.submit();\n" .
+        "//]]>\n" .
+        "</script>\n";
+
+    return $r;
+}
+
+/**
+ * Prepares an LTI 1.3 login request
+ *
+ * @param int            $courseid  Course ID
+ * @param int            $id        LTI instance ID
+ * @param stdClass|null  $instance  LTI instance
+ * @param stdClass       $config    Tool type configuration
+ * @param string         $messagetype   LTI message type
+ * @return array Login request parameters
+ */
+function lti_build_login_request($courseid, $id, $instance, $config, $messagetype) {
+    global $USER, $CFG;
 
     if (!empty($instance)) {
         $endpoint = !empty($instance->toolurl) ? $instance->toolurl : $config->lti_toolurl;
@@ -3302,27 +3339,9 @@ function lti_initiate_login($courseid, $id, $instance, $config, $messagetype = '
     $params['target_link_uri'] = $endpoint;
     $params['login_hint'] = $USER->id;
     $params['lti_message_hint'] = $id;
-    $SESSION->lti_message_hint = "{$courseid},{$config->typeid},{$id}," . base64_encode($title) . ',' .
-        base64_encode($text);
-
-    $r = "<form action=\"" . $config->lti_initiatelogin .
-        "\" name=\"ltiInitiateLoginForm\" id=\"ltiInitiateLoginForm\" method=\"post\" " .
-        "encType=\"application/x-www-form-urlencoded\">\n";
-
-    foreach ($params as $key => $value) {
-        $key = htmlspecialchars($key);
-        $value = htmlspecialchars($value);
-        $r .= "  <input type=\"hidden\" name=\"{$key}\" value=\"{$value}\"/>\n";
-    }
-    $r .= "</form>\n";
-
-    $r .= "<script type=\"text/javascript\">\n" .
-        "//<![CDATA[\n" .
-        "document.ltiInitiateLoginForm.submit();\n" .
-        "//]]>\n" .
-        "</script>\n";
-
-    return $r;
+    $params['client_id'] = $config->lti_clientid;
+    $params['lti_deployment_id'] = $config->typeid;
+    return $params;
 }
 
 function lti_get_type($typeid) {
index f79359a..d9c90ca 100644 (file)
@@ -1383,4 +1383,36 @@ MwIDAQAB
         $this->assertEquals($token->timecreated + LTI_ACCESS_TOKEN_LIFE, $token->validuntil);
         $this->assertNull($token->lastaccess);
     }
+
+    /**
+     * Test lti_build_login_request().
+     */
+    public function test_lti_build_login_request() {
+        global $USER, $CFG;
+
+        $this->resetAfterTest();
+
+        $USER->id = 123456789;
+
+        $course   = $this->getDataGenerator()->create_course();
+        $instance = $this->getDataGenerator()->create_module('lti',
+            [
+                'course' => $course->id,
+            ]
+        );
+
+        $config = new stdClass();
+        $config->lti_clientid = 'some-client-id';
+        $config->typeid = 'some-type-id';
+        $config->lti_toolurl = 'some-lti-tool-url';
+
+        $request = lti_build_login_request($course->id, $instance->id, $instance, $config, 'basic-lti-launch-request');
+
+        $this->assertEquals($CFG->wwwroot, $request['iss']);
+        $this->assertEquals('http://some-lti-tool-url', $request['target_link_uri']);
+        $this->assertEquals(123456789, $request['login_hint']);
+        $this->assertEquals($instance->id, $request['lti_message_hint']);
+        $this->assertEquals('some-client-id', $request['client_id']);
+        $this->assertEquals('some-type-id', $request['lti_deployment_id']);
+    }
 }
index 128db02..eb467b9 100644 (file)
@@ -39,7 +39,7 @@ require_once("$CFG->dirroot/mod/url/lib.php");
 function url_appears_valid_url($url) {
     if (preg_match('/^(\/|https?:|ftp:)/i', $url)) {
         // note: this is not exact validation, we look for severely malformed URLs only
-        return (bool)preg_match('/^[a-z]+:\/\/([^:@\s]+:[^@\s]+@)?[a-z0-9_\.\-]+(:[0-9]+)?(\/[^#]*)?(#.*)?$/i', $url);
+        return (bool) preg_match('/^[a-z]+:\/\/([^:@\s]+:[^@\s]+@)?[^ @]+(:[0-9]+)?(\/[^#]*)?(#.*)?$/i', $url);
     } else {
         return (bool)preg_match('/^[a-z]+:\/\/...*$/i', $url);
     }
@@ -88,10 +88,23 @@ function url_get_full_url($url, $cm, $course, $config=null) {
     // make sure there are no encoded entities, it is ok to do this twice
     $fullurl = html_entity_decode($url->externalurl, ENT_QUOTES, 'UTF-8');
 
+    $letters = '\pL';
+    $latin = 'a-zA-Z';
+    $digits = '0-9';
+    $symbols = '\x{20E3}\x{00AE}\x{00A9}\x{203C}\x{2047}\x{2048}\x{2049}\x{3030}\x{303D}\x{2139}\x{2122}\x{3297}\x{3299}' .
+               '\x{2300}-\x{23FF}\x{2600}-\x{27BF}\x{2B00}-\x{2BF0}';
+    $arabic = '\x{FE00}-\x{FEFF}';
+    $math = '\x{2190}-\x{21FF}\x{2900}-\x{297F}';
+    $othernumbers = '\x{2460}-\x{24FF}';
+    $geometric = '\x{25A0}-\x{25FF}';
+    $emojis = '\x{1F000}-\x{1F6FF}';
+
     if (preg_match('/^(\/|https?:|ftp:)/i', $fullurl) or preg_match('|^/|', $fullurl)) {
         // encode extra chars in URLs - this does not make it always valid, but it helps with some UTF-8 problems
-        $allowed = "a-zA-Z0-9".preg_quote(';/?:@=&$_.+!*(),-#%', '/');
-        $fullurl = preg_replace_callback("/[^$allowed]/", 'url_filter_callback', $fullurl);
+        // Thanks to 💩.la emojis count as valid, too.
+        $allowed = "[" . $letters . $latin . $digits . $symbols . $arabic . $math . $othernumbers . $geometric .
+            $emojis . "]" . preg_quote(';/?:@=&$_.+!*(),-#%', '/');
+        $fullurl = preg_replace_callback("/[^$allowed]/u", 'url_filter_callback', $fullurl);
     } else {
         // encode special chars only
         $fullurl = str_replace('"', '%22', $fullurl);
index 76e6d65..50198f0 100644 (file)
@@ -53,6 +53,25 @@ class mod_url_lib_testcase extends advanced_testcase {
     public function test_url_appears_valid_url() {
         $this->assertTrue(url_appears_valid_url('http://example'));
         $this->assertTrue(url_appears_valid_url('http://www.example.com'));
+        $this->assertTrue(url_appears_valid_url('http://www.examplé.com'));
+        $this->assertTrue(url_appears_valid_url('http://💩.la'));
+        $this->assertTrue(url_appears_valid_url('http://香港大學.香港'));
+        $this->assertTrue(url_appears_valid_url('http://وزارة-الأتصالات.مصر'));
+        $this->assertTrue(url_appears_valid_url('http://www.теннис-алт.рф'));
+        $this->assertTrue(url_appears_valid_url('http://имена.бг'));
+        $this->assertTrue(url_appears_valid_url('http://straße.de'));
+        $this->assertTrue(url_appears_valid_url('http://キース.コム'));
+        $this->assertTrue(url_appears_valid_url('http://太亞.中国'));
+        $this->assertTrue(url_appears_valid_url('http://www.რეგისტრაცია.გე'));
+        $this->assertTrue(url_appears_valid_url('http://уміц.укр'));
+        $this->assertTrue(url_appears_valid_url('http://현대.한국'));
+        $this->assertTrue(url_appears_valid_url('http://мон.мон'));
+        $this->assertTrue(url_appears_valid_url('http://тест.қаз'));
+        $this->assertTrue(url_appears_valid_url('http://рнидс.срб'));
+        $this->assertTrue(url_appears_valid_url('http://اسماء.شبكة'));
+        $this->assertTrue(url_appears_valid_url('http://www.informationssäkerhet.se'));
+        $this->assertTrue(url_appears_valid_url('http://москва.рф/services'));
+        $this->assertTrue(url_appears_valid_url('http://detdumærker.dk'));
         $this->assertTrue(url_appears_valid_url('http://www.exa-mple2.com'));
         $this->assertTrue(url_appears_valid_url('http://www.example.com/~nobody/index.html'));
         $this->assertTrue(url_appears_valid_url('http://www.example.com#hmm'));
@@ -60,6 +79,7 @@ class mod_url_lib_testcase extends advanced_testcase {
         $this->assertTrue(url_appears_valid_url('http://www.example.com/žlutý koníček/lala.txt'));
         $this->assertTrue(url_appears_valid_url('http://www.example.com/žlutý koníček/lala.txt#hmmmm'));
         $this->assertTrue(url_appears_valid_url('http://www.example.com/index.php?xx=yy&zz=aa'));
+        $this->assertTrue(url_appears_valid_url('http://www.example.com:80/index.php?xx=yy&zz=aa'));
         $this->assertTrue(url_appears_valid_url('https://user:password@www.example.com/žlutý koníček/lala.txt'));
         $this->assertTrue(url_appears_valid_url('ftp://user:password@www.example.com/žlutý koníček/lala.txt'));
 
@@ -67,7 +87,6 @@ class mod_url_lib_testcase extends advanced_testcase {
         $this->assertFalse(url_appears_valid_url('http:/example.com'));
         $this->assertFalse(url_appears_valid_url('http://'));
         $this->assertFalse(url_appears_valid_url('http://www.exa mple.com'));
-        $this->assertFalse(url_appears_valid_url('http://www.examplé.com'));
         $this->assertFalse(url_appears_valid_url('http://@www.example.com'));
         $this->assertFalse(url_appears_valid_url('http://user:@www.example.com'));
 
index 583a41b..255cbe7 100644 (file)
@@ -42,7 +42,7 @@ class qbehaviour_interactivecountback_walkthrough_test extends qbehaviour_walkth
     public function test_interactive_feedback_match_reset() {
 
         // Create a matching question.
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->shufflestems = false;
         $m->hints = array(
             new question_hint_with_parts(0, 'This is the first hint.', FORMAT_HTML, true, true),
index 9cbec70..072abe1 100644 (file)
@@ -86,7 +86,7 @@ class edit_menu_column extends column_base {
             }
         }
 
-        $qtypeactions = \question_bank::get_qtype($question->qtype)
+        $qtypeactions = \question_bank::get_qtype($question->qtype, false)
                 ->get_extra_question_bank_actions($question);
         foreach ($qtypeactions as $action) {
             $menu->add($action);
index d602098..8443178 100644 (file)
@@ -375,29 +375,7 @@ class test_question_maker {
      * @return qtype_match_question
      */
     public static function make_a_matching_question() {
-        question_bank::load_question_definition_classes('match');
-        $match = new qtype_match_question();
-        self::initialise_a_question($match);
-        $match->name = 'Matching question';
-        $match->questiontext = 'Classify the animals.';
-        $match->generalfeedback = 'Frogs and toads are amphibians, the others are mammals.';
-        $match->qtype = question_bank::get_qtype('match');
-
-        $match->shufflestems = 1;
-
-        self::set_standard_combined_feedback_fields($match);
-
-        // Using unset to get 1-based arrays.
-        $match->stems = array('', 'Dog', 'Frog', 'Toad', 'Cat');
-        $match->stemformat = array('', FORMAT_HTML, FORMAT_HTML, FORMAT_HTML, FORMAT_HTML);
-        $match->choices = array('', 'Mammal', 'Amphibian', 'Insect');
-        $match->right = array('', 1, 2, 2, 1);
-        unset($match->stems[0]);
-        unset($match->stemformat[0]);
-        unset($match->choices[0]);
-        unset($match->right[0]);
-
-        return $match;
+        return self::make_question('match');
     }
 
     /**
@@ -431,7 +409,7 @@ class test_question_maker {
      * Add some standard overall feedback to a question. You need to use these
      * specific feedback strings for the corresponding contains_..._feedback
      * methods in {@link qbehaviour_walkthrough_test_base} to works.
-     * @param question_definition $q the question to add the feedback to.
+     * @param question_definition|stdClass $q the question to add the feedback to.
      */
     public static function set_standard_combined_feedback_fields($q) {
         $q->correctfeedback = self::STANDARD_OVERALL_CORRECT_FEEDBACK;
index 962c56f..ca4cba7 100644 (file)
@@ -178,7 +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->idnumber = core_question_find_next_unused_idnumber($question->idnumber, $category->id);
         $question->beingcopied = true;
     }
 
index 0f4ea44..e2c77fa 100644 (file)
@@ -70,4 +70,35 @@ class core_question_bank_view_testcase extends advanced_testcase {
         // Verify the question has not been loaded into the cache.
         $this->assertFalse($cache->has($questiondata->id));
     }
+
+    public function test_unknown_qtype_does_not_break_view() {
+        global $DB;
+        $this->resetAfterTest();
+        $this->setAdminUser();
+        $generator = $this->getDataGenerator();
+        /** @var core_question_generator $questiongenerator */
+        $questiongenerator = $generator->get_plugin_generator('core_question');
+
+        // Create a course.
+        $course = $generator->create_course();
+        $context = context_course::instance($course->id);
+
+        // Create a question in the default category.
+        $contexts = new question_edit_contexts($context);
+        $cat = question_make_default_categories($contexts->all());
+        $questiondata = $questiongenerator->create_question('numerical', null,
+                ['name' => 'Example question', 'category' => $cat->id]);
+        $DB->set_field('question', 'qtype', 'unknownqtype', ['id' => $questiondata->id]);
+
+        // Generate the view.
+        $view = new core_question\bank\view($contexts, new moodle_url('/'), $course);
+        ob_start();
+        $view->display('editq', 0, 20, $cat->id . ',' . $context->id, false, false, false);
+        $html = ob_get_clean();
+
+        // Mainly we are verifying that there was no fatal error.
+
+        // Verify the output includes the expected question.
+        $this->assertContains('Example question', $html);
+    }
 }
index 4113010..6aba21d 100644 (file)
@@ -46,6 +46,8 @@ class qtype_match_question extends question_graded_automatically_with_countback
 
     /** @var array of question stems. */
     public $stems;
+    /** @var int[] FORMAT_... type for each stem. */
+    public $stemformat;
     /** @var array of choices that can be matched to each stem. */
     public $choices;
     /** @var array index of the right choice for each stem. */
index ffc9b4d..7e35d45 100644 (file)
@@ -124,8 +124,7 @@ class qtype_match extends question_type {
         $question->right = array();
 
         foreach ($questiondata->options->subquestions as $matchsub) {
-            $ans = $matchsub->answertext;
-            $key = array_search($matchsub->answertext, $question->choices);
+            $key = array_search($matchsub->answertext, $question->choices, true);
             if ($key === false) {
                 $key = $matchsub->id;
                 $question->choices[$key] = $matchsub->answertext;
diff --git a/question/type/match/tests/backup_test.php b/question/type/match/tests/backup_test.php
new file mode 100644 (file)
index 0000000..ec34d0b
--- /dev/null
@@ -0,0 +1,87 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Tests for the matching question type backup and restore logic.
+ *
+ * @package   qtype_match
+ * @copyright 2020 The Open University
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later.
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
+require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php');
+require_once($CFG->dirroot . '/course/externallib.php');
+
+
+/**
+ * Tests for the matching question type backup and restore logic.
+ */
+class qtype_match_backup_testcase extends advanced_testcase {
+
+    /**
+     * Duplicate quiz with a matching question, and check it worked.
+     */
+    public function test_duplicate_match_question() {
+        global $DB;
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $coregenerator = $this->getDataGenerator();
+        $questiongenerator = $coregenerator->get_plugin_generator('core_question');
+
+        // Create a course with a page that embeds a question.
+        $course = $coregenerator->create_course();
+        $quiz = $coregenerator->create_module('quiz', ['course' => $course->id]);
+        $quizcontext = context_module::instance($quiz->cmid);
+
+        $cat = $questiongenerator->create_question_category(['contextid' => $quizcontext->id]);
+        $question = $questiongenerator->create_question('match', 'trickynums', ['category' => $cat->id]);
+
+        // Store some counts.
+        $numquizzes = count(get_fast_modinfo($course)->instances['quiz']);
+        $nummatchquestions = $DB->count_records('question', ['qtype' => 'match']);
+
+        // Duplicate the page.
+        duplicate_module($course, get_fast_modinfo($course)->get_cm($quiz->cmid));
+
+        // Verify the copied quiz exists.
+        $this->assertCount($numquizzes + 1, get_fast_modinfo($course)->instances['quiz']);
+
+        // Verify the copied question.
+        $this->assertEquals($nummatchquestions + 1, $DB->count_records('question', ['qtype' => 'match']));
+        $newmatchid = $DB->get_field_sql("
+                SELECT MAX(id)
+                  FROM {question}
+                 WHERE qtype = ?
+                ", ['match']);
+        $matchdata = question_bank::load_question_data($newmatchid);
+
+        $subquestions = array_values($matchdata->options->subquestions);
+
+        $this->assertSame('System.out.println(0);', $subquestions[0]->questiontext);
+        $this->assertSame('0', $subquestions[0]->answertext);
+
+        $this->assertSame('System.out.println(0.0);', $subquestions[1]->questiontext);
+        $this->assertSame('0.0', $subquestions[1]->answertext);
+
+        $this->assertSame('', $subquestions[2]->questiontext);
+        $this->assertSame('NULL', $subquestions[2]->answertext);
+    }
+}
index 1e0aabb..b93f3da 100644 (file)
@@ -37,10 +37,9 @@ require_once($CFG->dirroot . '/question/type/match/question.php');
  */
 class qtype_match_test_helper extends question_test_helper {
     public function get_test_questions() {
-        return array('foursubq');
+        return array('foursubq', 'trickynums');
     }
 
-
     /**
      * Makes a match question about completing two blanks in some text.
      * @return object the question definition data, as it might be returned from
@@ -128,4 +127,149 @@ class qtype_match_test_helper extends question_test_helper {
         return $q;
     }
 
+    /**
+     * Makes a matching question to classify 'Dog', 'Frog', 'Toad' and 'Cat' as
+     * 'Mammal', 'Amphibian' or 'Insect'.
+     * defaultmark 1. Stems are shuffled by default.
+     * @return qtype_match_question
+     */
+    public static function make_match_question_foursubq() {
+        question_bank::load_question_definition_classes('match');
+        $match = new qtype_match_question();
+        test_question_maker::initialise_a_question($match);
+        $match->name = 'Matching question';
+        $match->questiontext = 'Classify the animals.';
+        $match->generalfeedback = 'Frogs and toads are amphibians, the others are mammals.';
+        $match->qtype = question_bank::get_qtype('match');
+
+        $match->shufflestems = 1;
+
+        test_question_maker::set_standard_combined_feedback_fields($match);
+
+        // Using unset to get 1-based arrays.
+        $match->stems = array('', 'Dog', 'Frog', 'Toad', 'Cat');
+        $match->stemformat = array('', FORMAT_HTML, FORMAT_HTML, FORMAT_HTML, FORMAT_HTML);
+        $match->choices = array('', 'Mammal', 'Amphibian', 'Insect');
+        $match->right = array('', 1, 2, 2, 1);
+        unset($match->stems[0]);
+        unset($match->stemformat[0]);
+        unset($match->choices[0]);
+        unset($match->right[0]);
+
+        return $match;
+    }
+
+    /**
+     * Makes a matching question with choices including '0' and '0.0'.
+     *
+     * @return object the question definition data, as it might be returned from
+     * get_question_options.
+     */
+    public function get_match_question_data_trickynums() {
+        global $USER;
+
+        $q = new stdClass();
+        test_question_maker::initialise_question_data($q);
+        $q->name = 'Java matching';
+        $q->qtype = 'match';
+        $q->parent = 0;
+        $q->questiontext = 'What is the output of each of these lines of code?';
+        $q->questiontextformat = FORMAT_HTML;
+        $q->generalfeedback = 'Java has some advantages over PHP I guess!';
+        $q->generalfeedbackformat = FORMAT_HTML;
+        $q->defaultmark = 1;
+        $q->penalty = 0.3333333;
+        $q->length = 1;
+        $q->hidden = 0;
+        $q->createdby = $USER->id;
+        $q->modifiedby = $USER->id;
+
+        $q->options = new stdClass();
+        $q->options->shuffleanswers = 1;
+        test_question_maker::set_standard_combined_feedback_fields($q->options);
+
+        $q->options->subquestions = array(
+                14 => (object) array(
+                        'id' => 14,
+                        'questiontext' => 'System.out.println(0);',
+                        'questiontextformat' => FORMAT_HTML,
+                        'answertext' => '0'),
+                15 => (object) array(
+                        'id' => 15,
+                        'questiontext' => 'System.out.println(0.0);',
+                        'questiontextformat' => FORMAT_HTML,
+                        'answertext' => '0.0'),
+                16 => (object) array(
+                        'id' => 16,
+                        'questiontext' => '',
+                        'questiontextformat' => FORMAT_HTML,
+                        'answertext' => 'NULL'),
+        );
+
+        return $q;
+    }
+
+    /**
+     * Makes a match question about completing two blanks in some text.
+     * @return object the question definition data, as it might be returned from
+     *      the question editing form.
+     */
+    public function get_match_question_form_data_trickynums() {
+        $q = new stdClass();
+        $q->name = 'Java matching';
+        $q->questiontext = ['text' => 'What is the output of each of these lines of code?', 'format' => FORMAT_HTML];
+        $q->generalfeedback = ['text' => 'Java has some advantages over PHP I guess!', 'format' => FORMAT_HTML];
+        $q->defaultmark = 1;
+        $q->penalty = 0.3333333;
+
+        $q->shuffleanswers = 1;
+        test_question_maker::set_standard_combined_feedback_form_data($q);
+
+        $q->subquestions = array(
+            0 => array('text' => 'System.out.println(0);', 'format' => FORMAT_HTML),
+            1 => array('text' => 'System.out.println(0.0);', 'format' => FORMAT_HTML),
+            2 => array('text' => '', 'format' => FORMAT_HTML),
+        );
+
+        $q->subanswers = array(
+            0 => '0',
+            1 => '0.0',
+            2 => 'NULL',
+        );
+
+        $q->noanswers = 3;
+
+        return $q;
+    }
+
+    /**
+     * Makes a matching question with choices including '0' and '0.0'.
+     *
+     * @return qtype_match_question
+     */
+    public static function make_match_question_trickynums() {
+        question_bank::load_question_definition_classes('match');
+        $match = new qtype_match_question();
+        test_question_maker::initialise_a_question($match);
+        $match->name = 'Java matching';
+        $match->questiontext = 'What is the output of each of these lines of code?';
+        $match->generalfeedback = 'Java has some advantages over PHP I guess!';
+        $match->qtype = question_bank::get_qtype('match');
+
+        $match->shufflestems = 1;
+
+        test_question_maker::set_standard_combined_feedback_fields($match);
+
+        // Using unset to get 1-based arrays.
+        $match->stems = array('', 'System.out.println(0);', 'System.out.println(0.0);');
+        $match->stemformat = array('', FORMAT_HTML, FORMAT_HTML);
+        $match->choices = array('', '0', '0.0', 'NULL');
+        $match->right = array('', 1, 2);
+        unset($match->stems[0]);
+        unset($match->stemformat[0]);
+        unset($match->choices[0]);
+        unset($match->right[0]);
+
+        return $match;
+    }
 }
index 4e83431..67ab401 100644 (file)
@@ -38,7 +38,7 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
 class qtype_match_question_test extends advanced_testcase {
 
     public function test_get_expected_data() {
-        $question = test_question_maker::make_a_matching_question();
+        $question = test_question_maker::make_question('match');
         $question->start_attempt(new question_attempt_step(), 1);
 
         $this->assertEquals(array('sub0' => PARAM_INT, 'sub1' => PARAM_INT,
@@ -46,7 +46,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_is_complete_response() {
-        $question = test_question_maker::make_a_matching_question();
+        $question = test_question_maker::make_question('match');
         $question->start_attempt(new question_attempt_step(), 1);
 
         $this->assertFalse($question->is_complete_response(array()));
@@ -58,7 +58,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_is_gradable_response() {
-        $question = test_question_maker::make_a_matching_question();
+        $question = test_question_maker::make_question('match');
         $question->start_attempt(new question_attempt_step(), 1);
 
         $this->assertFalse($question->is_gradable_response(array()));
@@ -72,7 +72,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_is_same_response() {
-        $question = test_question_maker::make_a_matching_question();
+        $question = test_question_maker::make_question('match');
         $question->start_attempt(new question_attempt_step(), 1);
 
         $this->assertTrue($question->is_same_response(
@@ -97,7 +97,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_grading() {
-        $question = test_question_maker::make_a_matching_question();
+        $question = test_question_maker::make_question('match');
         $question->start_attempt(new question_attempt_step(), 1);
 
         $correctresponse = $question->prepare_simulated_post_data(
@@ -126,7 +126,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_get_correct_response() {
-        $question = test_question_maker::make_a_matching_question();
+        $question = test_question_maker::make_question('match');
         $question->start_attempt(new question_attempt_step(), 1);
 
         $correct = $question->prepare_simulated_post_data(array('Dog' => 'Mammal',
@@ -137,7 +137,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_get_question_summary() {
-        $match = test_question_maker::make_a_matching_question();
+        $match = test_question_maker::make_question('match');
         $match->start_attempt(new question_attempt_step(), 1);
         $qsummary = $match->get_question_summary();
         $this->assertRegExp('/' . preg_quote($match->questiontext, '/') . '/', $qsummary);
@@ -150,7 +150,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_summarise_response() {
-        $match = test_question_maker::make_a_matching_question();
+        $match = test_question_maker::make_question('match');
         $match->start_attempt(new question_attempt_step(), 1);
 
         $summary = $match->summarise_response($match->prepare_simulated_post_data(array('Dog' => 'Amphibian', 'Frog' => 'Mammal')));
@@ -160,7 +160,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_classify_response() {
-        $match = test_question_maker::make_a_matching_question();
+        $match = test_question_maker::make_question('match');
         $match->start_attempt(new question_attempt_step(), 1);
 
         $response = $match->prepare_simulated_post_data(array('Dog' => 'Amphibian', 'Frog' => 'Insect', 'Toad' => '', 'Cat' => ''));
@@ -182,14 +182,14 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_classify_response_choice_deleted_after_attempt() {
-        $match = test_question_maker::make_a_matching_question();
+        $match = test_question_maker::make_question('match');
         $firststep = new question_attempt_step();
 
         $match->start_attempt($firststep, 1);
         $response = $match->prepare_simulated_post_data(array(
                 'Dog' => 'Amphibian', 'Frog' => 'Insect', 'Toad' => '', 'Cat' => 'Mammal'));
 
-        $match = test_question_maker::make_a_matching_question();
+        $match = test_question_maker::make_question('match');
         unset($match->stems[4]);
         unset($match->stemsformat[4]);
         unset($match->right[4]);
@@ -203,14 +203,14 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_classify_response_choice_added_after_attempt() {
-        $match = test_question_maker::make_a_matching_question();
+        $match = test_question_maker::make_question('match');
         $firststep = new question_attempt_step();
 
         $match->start_attempt($firststep, 1);
         $response = $match->prepare_simulated_post_data(array(
                 'Dog' => 'Amphibian', 'Frog' => 'Insect', 'Toad' => '', 'Cat' => 'Mammal'));
 
-        $match = test_question_maker::make_a_matching_question();
+        $match = test_question_maker::make_question('match');
         $match->stems[5] = "Snake";
         $match->stemsformat[5] = FORMAT_HTML;
         $match->choices[5] = "Reptile";
@@ -226,7 +226,7 @@ class qtype_match_question_test extends advanced_testcase {
     }
 
     public function test_prepare_simulated_post_data() {
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->start_attempt(new question_attempt_step(), 1);
         $postdata = $m->prepare_simulated_post_data(array('Dog' => 'Mammal', 'Frog' => 'Amphibian',
                                                           'Toad' => 'Amphibian', 'Cat' => 'Mammal'));
index d60a0b4..baaf44d 100644 (file)
@@ -112,6 +112,26 @@ class qtype_match_test extends advanced_testcase {
         $this->assertTrue($this->qtype->can_analyse_responses());
     }
 
+    public function test_make_question_instance() {
+        $questiondata = test_question_maker::get_question_data('match', 'trickynums');
+        $question = question_bank::make_question($questiondata);
+        $this->assertEquals($questiondata->name, $question->name);
+        $this->assertEquals($questiondata->questiontext, $question->questiontext);
+        $this->assertEquals($questiondata->questiontextformat, $question->questiontextformat);
+        $this->assertEquals($questiondata->generalfeedback, $question->generalfeedback);
+        $this->assertEquals($questiondata->generalfeedbackformat, $question->generalfeedbackformat);
+        $this->assertInstanceOf('qtype_match', $question->qtype);
+        $this->assertEquals($questiondata->options->shuffleanswers, $question->shufflestems);
+
+        $this->assertEquals(
+                [14 => 'System.out.println(0);', 15 => 'System.out.println(0.0);'],
+                $question->stems);
+
+        $this->assertEquals([14 => '0', 15 => '0.0', 16 => 'NULL'], $question->choices);
+
+        $this->assertEquals([14 => 14, 15 => 15], $question->right);
+    }
+
     public function test_get_random_guess_score() {
         $q = $this->get_test_question_data();
         $this->assertEquals(0.3333333, $this->qtype->get_random_guess_score($q), '', 0.0000001);
index f5bd7af..b793040 100644 (file)
@@ -41,7 +41,7 @@ class qtype_match_walkthrough_test extends qbehaviour_walkthrough_test_base {
     public function test_deferred_feedback_unanswered() {
 
         // Create a matching question.
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->shufflestems = false;
         $this->start_attempt_at_question($m, 'deferredfeedback', 4);
 
@@ -98,7 +98,7 @@ class qtype_match_walkthrough_test extends qbehaviour_walkthrough_test_base {
     public function test_deferred_feedback_partial_answer() {
 
         // Create a matching question.
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->shufflestems = false;
         $this->start_attempt_at_question($m, 'deferredfeedback', 4);
 
@@ -155,7 +155,7 @@ class qtype_match_walkthrough_test extends qbehaviour_walkthrough_test_base {
     public function test_interactive_correct_no_submit() {
 
         // Create a matching question.
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->hints = array(
             new question_hint_with_parts(11, 'This is the first hint.', FORMAT_HTML, false, false),
             new question_hint_with_parts(12, 'This is the second hint.', FORMAT_HTML, true, true),
@@ -209,7 +209,7 @@ class qtype_match_walkthrough_test extends qbehaviour_walkthrough_test_base {
     public function test_interactive_partial_no_submit() {
 
         // Create a matching question.
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->hints = array(
             new question_hint_with_parts(11, 'This is the first hint.', FORMAT_HTML, false, false),
             new question_hint_with_parts(12, 'This is the second hint.', FORMAT_HTML, true, true),
@@ -263,7 +263,7 @@ class qtype_match_walkthrough_test extends qbehaviour_walkthrough_test_base {
     public function test_interactive_with_invalid() {
 
         // Create a matching question.
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->hints = array(
             new question_hint_with_parts(11, 'This is the first hint.', FORMAT_HTML, false, false),
             new question_hint_with_parts(12, 'This is the second hint.', FORMAT_HTML, true, true),
@@ -333,7 +333,7 @@ class qtype_match_walkthrough_test extends qbehaviour_walkthrough_test_base {
     public function test_match_with_tricky_html_choices() {
 
         // Create a matching question.
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->stems = array(
             1 => '(1, 2]',
             2 => '[1, 2]',
@@ -387,7 +387,7 @@ class qtype_match_walkthrough_test extends qbehaviour_walkthrough_test_base {
     public function test_match_clear_wrong() {
 
         // Create a matching question.
-        $m = test_question_maker::make_a_matching_question();
+        $m = test_question_maker::make_question('match');
         $m->hints = array(
             new question_hint_with_parts(11, 'This is the first hint.', FORMAT_HTML, false, true),
             new question_hint_with_parts(12, 'This is the second hint.', FORMAT_HTML, true, true),
index f172541..d056d90 100644 (file)
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2020032000.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2020032000.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 $release  = '3.9dev (Build: 20200320)'; // Human-friendly version name