Moodle release 3.7.7

The behat hooks were needlessly complicated which made them much harder
to read, and understand, leading to bugs during development.

These have been significantly simplified to favour clarity over
overloading.

.eslintignore

@@ -65,6 +65,7 @@ lib/validateurlsyntax.php
 lib/amd/src/popper.js
 lib/geopattern-php/
 lib/php-jwt/
+lib/mdn-polyfills/
 media/player/videojs/amd/src/video-lazy.js
 media/player/videojs/amd/src/Youtube-lazy.js
 media/player/videojs/videojs/

.nvmrc

@@ -1 +1 @@
-v8.16.1
+v14.0.0

.stylelintignore

@@ -66,6 +66,7 @@ lib/validateurlsyntax.php
 lib/amd/src/popper.js
 lib/geopattern-php/
 lib/php-jwt/
+lib/mdn-polyfills/
 media/player/videojs/amd/src/video-lazy.js
 media/player/videojs/amd/src/Youtube-lazy.js
 media/player/videojs/videojs/

.travis.yml

@@ -2,8 +2,6 @@
 # process (which uses our internal CI system) this file is here for the benefit
 # of community developers git clones - see MDL-51458.
 
-sudo: required
-
 # We currently disable Travis notifications entirely until https://github.com/travis-ci/travis-ci/issues/4976
 # is fixed.
 notifications:
@@ -11,10 +9,13 @@ notifications:
 
 language: php
 
+os: linux
+
 dist: xenial
 
 services:
     - mysql
+    - docker
 
 php:
     # We only run the highest and lowest supported versions to reduce the load on travis-ci.org.
@@ -41,7 +42,7 @@ env:
     # Perform an upgrade test too.
     - DB=pgsql    TASK=UPGRADE
 
-matrix:
+jobs:
     # Enable fast finish.
     # This will fail the build if a single job fails (except those in allow_failures).
     # It will not stop the jobs from running.
@@ -61,8 +62,9 @@ cache:
       - $HOME/.npm
 
 before_install:
+    - docker run -d -p 127.0.0.1:8080:80 --name exttests moodlehq/moodle-exttests
     # Avoid IPv6 default binding as service (causes redis not to start).
-    sudo service redis-server start --bind 127.0.0.1
+    - sudo service redis-server start --bind 127.0.0.1
 
 install:
     - >
@@ -169,6 +171,10 @@ before_script:
           -e "/require_once/i \\\$CFG->phpunit_dataroot = '\/home\/travis\/roots\/phpunit';" \
           -e "/require_once/i \\\$CFG->phpunit_prefix = 'p_';" \
           config.php ;
+        # Enable test external resources
+        sed -i \
+          -e "/require_once/i \\define('TEST_EXTERNAL_FILES_HTTP_URL', 'http://127.0.0.1:8080');" \
+          config.php ;
         # Redis cache store tests
         sed -i \
           -e "/require_once/i \\define('TEST_CACHESTORE_REDIS_TESTSERVERS', '127.0.0.1');" \
@@ -248,6 +254,8 @@ script:
       if [ "$TASK" = 'PHPUNIT' ];
       then
         vendor/bin/phpunit --fail-on-risky --disallow-test-output --verbose;
+        EXTTESTS_HITS=$(docker logs exttests 2>&1 | grep -Fv -e 'AH00558' -e '[pid 1]' | wc -l)
+        echo -e "\nTest local resources number of hits: ${EXTTESTS_HITS}.\n"
       fi
 
     - >

Gruntfile.js

@@ -112,7 +112,13 @@ module.exports = function(grunt) {
         eslint: {
             // Even though warnings dont stop the build we don't display warnings by default because
             // at this moment we've got too many core warnings.
-            options: {quiet: !grunt.option('show-lint-warnings')},
+            // To display warnings call: grunt eslint --show-lint-warnings
+            // To fail on warnings call: grunt eslint --max-lint-warnings=0
+            // Also --max-lint-warnings=-1 can be used to display warnings but not fail.
+            options: {
+                quiet: (!grunt.option('show-lint-warnings')) && (typeof grunt.option('max-lint-warnings') === 'undefined'),
+                maxWarnings: ((typeof grunt.option('max-lint-warnings') !== 'undefined') ? grunt.option('max-lint-warnings') : -1)
+            },
             amd: {src: amdSrc},
             // Check YUI module source files.
             yui: {src: ['**/yui/src/**/*.js', '!*/**/yui/src/*/meta/*.js']}
@@ -151,6 +157,9 @@ module.exports = function(grunt) {
                 tasks: ['yui']
             },
             gherkinlint: {
+                options: {
+                    nospawn: false,
+                },
                 files: ['**/tests/behat/*.feature'],
                 tasks: ['gherkinlint']
             }
@@ -293,19 +302,27 @@ module.exports = function(grunt) {
     };
 
     tasks.gherkinlint = function() {
-        var done = this.async(),
-            options = grunt.config('gherkinlint.options');
+        const done = this.async();
+        const options = grunt.config('gherkinlint.options');
 
-        var args = grunt.file.expand(options.files);
-        args.unshift(path.normalize(__dirname + '/node_modules/.bin/gherkin-lint'));
-        grunt.util.spawn({
-            cmd: 'node',
-            args: args,
-            opts: {stdio: 'inherit', env: process.env}
-        }, function(error, result, code) {
-            // Propagate the exit code.
-            done(code === 0);
-        });
+        // Grab the gherkin-lint linter and required scaffolding.
+        const linter = require('gherkin-lint/src/linter.js');
+        const featureFinder = require('gherkin-lint/src/feature-finder.js');
+        const configParser = require('gherkin-lint/src/config-parser.js');
+        const formatter = require('gherkin-lint/src/formatters/stylish.js');
+
+        // Run the linter.
+        const results = linter.lint(
+            featureFinder.getFeatureFiles(grunt.file.expand(options.files)),
+            configParser.getConfiguration(configParser.defaultConfigFileName)
+        );
+
+        // Print the results out uncondtionally.
+        formatter.printResults(results);
+
+        // Report on the results.
+        // The done function takes a bool whereby a falsey statement causes the task to fail.
+        done(results.every(result => result.errors.length === 0));
     };
 
     tasks.startup = function() {

admin/classes/task_log_table.php

@@ -131,7 +131,8 @@ class task_log_table extends \table_sql {
         }
 
         $sql = "SELECT
-                    tl.*,
+                    tl.id, tl.type, tl.component, tl.classname, tl.userid, tl.timestart, tl.timeend,
+                    tl.dbreads, tl.dbwrites, tl.result,
                     tl.dbreads + tl.dbwrites AS db,
                     tl.timeend - tl.timestart AS duration,
                     {$userfields}

admin/cli/install.php

@@ -514,100 +514,105 @@ if ($interactive) {
 $database = $databases[$CFG->dbtype];
 
 
-// ask for db host
-if ($interactive) {
-    cli_separator();
-    cli_heading(get_string('databasehost', 'install'));
-    if ($options['dbhost'] !== '') {
-        $prompt = get_string('clitypevaluedefault', 'admin', $options['dbhost']);
+// We cannot do any validation until all DB connection data is provided.
+$hintdatabase = '';
+do {
+    echo $hintdatabase;
+
+    // Ask for db host.
+    if ($interactive) {
+        cli_separator();
+        cli_heading(get_string('databasehost', 'install'));
+        if ($options['dbhost'] !== '') {
+            $prompt = get_string('clitypevaluedefault', 'admin', $options['dbhost']);
+        } else {
+            $prompt = get_string('clitypevalue', 'admin');
+        }
+        $CFG->dbhost = cli_input($prompt, $options['dbhost']);
+
     } else {
-        $prompt = get_string('clitypevalue', 'admin');
+        $CFG->dbhost = $options['dbhost'];
     }
-    $CFG->dbhost = cli_input($prompt, $options['dbhost']);
 
-} else {
-    $CFG->dbhost = $options['dbhost'];
-}
+    // Ask for db name.
+    if ($interactive) {
+        cli_separator();
+        cli_heading(get_string('databasename', 'install'));
+        if ($options['dbname'] !== '') {
+            $prompt = get_string('clitypevaluedefault', 'admin', $options['dbname']);
+        } else {
+            $prompt = get_string('clitypevalue', 'admin');
+        }
+        $CFG->dbname = cli_input($prompt, $options['dbname']);
 
-// ask for db name
-if ($interactive) {
-    cli_separator();
-    cli_heading(get_string('databasename', 'install'));
-    if ($options['dbname'] !== '') {
-        $prompt = get_string('clitypevaluedefault', 'admin', $options['dbname']);
     } else {
-        $prompt = get_string('clitypevalue', 'admin');
+        $CFG->dbname = $options['dbname'];
     }
-    $CFG->dbname = cli_input($prompt, $options['dbname']);
 
-} else {
-    $CFG->dbname = $options['dbname'];
-}
+    // Ask for db prefix.
+    if ($interactive) {
+        cli_separator();
+        cli_heading(get_string('dbprefix', 'install'));
+        //TODO: solve somehow the prefix trouble for oci.
+        if ($options['prefix'] !== '') {
+            $prompt = get_string('clitypevaluedefault', 'admin', $options['prefix']);
+        } else {
+            $prompt = get_string('clitypevalue', 'admin');
+        }
+        $CFG->prefix = cli_input($prompt, $options['prefix']);
 
-// ask for db prefix
-if ($interactive) {
-    cli_separator();
-    cli_heading(get_string('dbprefix', 'install'));
-    //TODO: solve somehow the prefix trouble for oci
-    if ($options['prefix'] !== '') {
-        $prompt = get_string('clitypevaluedefault', 'admin', $options['prefix']);
     } else {
-        $prompt = get_string('clitypevalue', 'admin');
+        $CFG->prefix = $options['prefix'];
     }
-    $CFG->prefix = cli_input($prompt, $options['prefix']);
 
-} else {
-    $CFG->prefix = $options['prefix'];
-}
+    // Ask for db port.
+    if ($interactive) {
+        cli_separator();
+        cli_heading(get_string('databaseport', 'install'));
+        $prompt = get_string('clitypevaluedefault', 'admin', $options['dbport']);
+        $CFG->dboptions['dbport'] = (int) cli_input($prompt, $options['dbport']);
 
-// ask for db port
-if ($interactive) {
-    cli_separator();
-    cli_heading(get_string('databaseport', 'install'));
-    $prompt = get_string('clitypevaluedefault', 'admin', $options['dbport']);
-    $CFG->dboptions['dbport'] = (int)cli_input($prompt, $options['dbport']);
-
-} else {
-    $CFG->dboptions['dbport'] = (int)$options['dbport'];
-}
-if ($CFG->dboptions['dbport'] <= 0) {
-    $CFG->dboptions['dbport'] = '';
-}
-
-// ask for db socket
-if ($CFG->ostype === 'WINDOWS') {
-    $CFG->dboptions['dbsocket'] = '';
-
-} else if ($interactive and empty($CFG->dboptions['dbport'])) {
-    cli_separator();
-    cli_heading(get_string('databasesocket', 'install'));
-    $prompt = get_string('clitypevaluedefault', 'admin', $options['dbsocket']);
-    $CFG->dboptions['dbsocket'] = cli_input($prompt, $options['dbsocket']);
-
-} else {
-    $CFG->dboptions['dbsocket'] = $options['dbsocket'];
-}
-
-// ask for db user
-if ($interactive) {
-    cli_separator();
-    cli_heading(get_string('databaseuser', 'install'));
-    if ($options['dbuser'] !== '') {
-        $prompt = get_string('clitypevaluedefault', 'admin', $options['dbuser']);
     } else {
-        $prompt = get_string('clitypevalue', 'admin');
+        $CFG->dboptions['dbport'] = (int) $options['dbport'];
+    }
+    if ($CFG->dboptions['dbport'] <= 0) {
+        $CFG->dboptions['dbport'] = '';
     }
-    $CFG->dbuser = cli_input($prompt, $options['dbuser']);
 
-} else {
-    $CFG->dbuser = $options['dbuser'];
-}
+    // Ask for db socket.
+    if ($CFG->ostype === 'WINDOWS') {
+        $CFG->dboptions['dbsocket'] = '';
+
+    } else if ($interactive and empty($CFG->dboptions['dbport'])) {
+        cli_separator();
+        cli_heading(get_string('databasesocket', 'install'));
+        $prompt = get_string('clitypevaluedefault', 'admin', $options['dbsocket']);
+        $CFG->dboptions['dbsocket'] = cli_input($prompt, $options['dbsocket']);
+
+    } else {
+        $CFG->dboptions['dbsocket'] = $options['dbsocket'];
+    }
+
+    // Ask for db user.
+    if ($interactive) {
+        cli_separator();
+        cli_heading(get_string('databaseuser', 'install'));
+        if ($options['dbuser'] !== '') {
+            $prompt = get_string('clitypevaluedefault', 'admin', $options['dbuser']);
+        } else {
+            $prompt = get_string('clitypevalue', 'admin');
+        }
+        $CFG->dbuser = cli_input($prompt, $options['dbuser']);
+
+    } else {
+        $CFG->dbuser = $options['dbuser'];
+    }
+
+    // Ask for db password.
+    if ($interactive) {
+        cli_separator();
+        cli_heading(get_string('databasepass', 'install'));
 
-// ask for db password
-if ($interactive) {
-    cli_separator();
-    cli_heading(get_string('databasepass', 'install'));
-    do {
         if ($options['dbpass'] !== '') {
             $prompt = get_string('clitypevaluedefault', 'admin', $options['dbpass']);
         } else {
@@ -615,19 +620,23 @@ if ($interactive) {
         }
 
         $CFG->dbpass = cli_input($prompt, $options['dbpass']);
-        if (function_exists('distro_pre_create_db')) { // Hook for distros needing to do something before DB creation
-            $distro = distro_pre_create_db($database, $CFG->dbhost, $CFG->dbuser, $CFG->dbpass, $CFG->dbname, $CFG->prefix, array('dbpersist'=>0, 'dbport'=>$CFG->dboptions['dbport'], 'dbsocket'=>$CFG->dboptions['dbsocket']), $distro);
+        if (function_exists('distro_pre_create_db')) { // Hook for distros needing to do something before DB creation.
+            $distro = distro_pre_create_db($database, $CFG->dbhost, $CFG->dbuser, $CFG->dbpass, $CFG->dbname, $CFG->prefix,
+                    array('dbpersist' => 0, 'dbport' => $CFG->dboptions['dbport'], 'dbsocket' => $CFG->dboptions['dbsocket']),
+                    $distro);
         }
-        $hint_database = install_db_validate($database, $CFG->dbhost, $CFG->dbuser, $CFG->dbpass, $CFG->dbname, $CFG->prefix, array('dbpersist'=>0, 'dbport'=>$CFG->dboptions['dbport'], 'dbsocket'=>$CFG->dboptions['dbsocket']));
-    } while ($hint_database !== '');
+        $hintdatabase = install_db_validate($database, $CFG->dbhost, $CFG->dbuser, $CFG->dbpass, $CFG->dbname, $CFG->prefix,
+                array('dbpersist' => 0, 'dbport' => $CFG->dboptions['dbport'], 'dbsocket' => $CFG->dboptions['dbsocket']));
 
-} else {
-    $CFG->dbpass = $options['dbpass'];
-    $hint_database = install_db_validate($database, $CFG->dbhost, $CFG->dbuser, $CFG->dbpass, $CFG->dbname, $CFG->prefix, array('dbpersist'=>0, 'dbport'=>$CFG->dboptions['dbport'], 'dbsocket'=>$CFG->dboptions['dbsocket']));
-    if ($hint_database !== '') {
-        cli_error(get_string('dbconnectionerror', 'install'));
+    } else {
+        $CFG->dbpass = $options['dbpass'];
+        $hintdatabase = install_db_validate($database, $CFG->dbhost, $CFG->dbuser, $CFG->dbpass, $CFG->dbname, $CFG->prefix,
+                array('dbpersist' => 0, 'dbport' => $CFG->dboptions['dbport'], 'dbsocket' => $CFG->dboptions['dbsocket']));
+        if ($hintdatabase !== '') {
+            cli_error(get_string('dbconnectionerror', 'install'));
+        }
     }
-}
+} while ($hintdatabase !== '');
 
 // ask for fullname
 if ($interactive) {

admin/environment.xml

@@ -2451,6 +2451,7 @@
       <VENDOR name="oracle" version="11.2" />
     </DATABASE>
     <PHP version="7.0.0" level="required">
+      <RESTRICT function="restrict_php_version_74" message="unsupportedphpversion74" />
     </PHP>
     <PCREUNICODE level="optional">
       <FEEDBACK>
@@ -2632,6 +2633,7 @@
       <VENDOR name="oracle" version="11.2" />
     </DATABASE>
     <PHP version="7.1.0" level="required">
+      <RESTRICT function="restrict_php_version_74" message="unsupportedphpversion74" />
     </PHP>
     <PCREUNICODE level="optional">
       <FEEDBACK>
@@ -2980,4 +2982,185 @@
       </CUSTOM_CHECK>
     </CUSTOM_CHECKS>
   </MOODLE>
+  <MOODLE version="3.9" requires="3.5">
+    <UNICODE level="required">
+      <FEEDBACK>
+        <ON_ERROR message="unicoderequired" />
+      </FEEDBACK>
+    </UNICODE>
+    <DATABASE level="required">
+      <VENDOR name="mariadb" version="10.2.29" />
+      <VENDOR name="mysql" version="5.6" />
+      <VENDOR name="postgres" version="9.5" />
+      <VENDOR name="mssql" version="11.0" />
+      <VENDOR name="oracle" version="11.2" />
+    </DATABASE>
+    <PHP version="7.2.0" level="required">
+    </PHP>
+    <PCREUNICODE level="optional">
+      <FEEDBACK>
+        <ON_CHECK message="pcreunicodewarning" />
+      </FEEDBACK>
+    </PCREUNICODE>
+    <PHP_EXTENSIONS>
+      <PHP_EXTENSION name="iconv" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="iconvrequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="mbstring" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="mbstringrequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="curl" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="curlrequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="openssl" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="opensslrequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="tokenizer" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="tokenizerrecommended" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="xmlrpc" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="xmlrpcrecommended" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="soap" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="soaprecommended" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="ctype" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="ctyperequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="zip" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="ziprequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="zlib" level="required">
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="gd" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="gdrequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="simplexml" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="simplexmlrequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="spl" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="splrequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="pcre" level="required">
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="dom" level="required">
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="xml" level="required">
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="xmlreader" level="required">
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="intl" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="intlrequired" />
+        </FEEDBACK>
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="json" level="required">
+      </PHP_EXTENSION>
+      <PHP_EXTENSION name="hash" level="required"/>
+      <PHP_EXTENSION name="fileinfo" level="required"/>
+    </PHP_EXTENSIONS>
+    <PHP_SETTINGS>
+      <PHP_SETTING name="memory_limit" value="96M" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="settingmemorylimit" />
+        </FEEDBACK>
+      </PHP_SETTING>
+      <PHP_SETTING name="file_uploads" value="1" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="settingfileuploads" />
+        </FEEDBACK>
+      </PHP_SETTING>
+      <PHP_SETTING name="opcache.enable" value="1" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="opcacherecommended" />
+        </FEEDBACK>
+      </PHP_SETTING>
+    </PHP_SETTINGS>
+    <CUSTOM_CHECKS>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_database_storage_engine" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="unsupporteddbstorageengine" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="question/engine/upgrade/upgradelib.php" function="quiz_attempts_upgraded" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="quizattemptsupgradedmessage" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_slasharguments" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="slashargumentswarning" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_database_tables_row_format" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="unsupporteddbtablerowformat" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_unoconv_version" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="unoconvwarning" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_libcurl_version" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="libcurlwarning" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_mysql_file_format" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="unsupporteddbfileformat" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_mysql_file_per_table" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="unsupporteddbfilepertable" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_mysql_large_prefix" level="required">
+        <FEEDBACK>
+          <ON_ERROR message="unsupporteddblargeprefix" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_is_https" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="ishttpswarning" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_mysql_incomplete_unicode_support" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="incompleteunicodesupport" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_sixtyfour_bits" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="sixtyfourbitswarning" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+    </CUSTOM_CHECKS>
+  </MOODLE>
 </COMPATIBILITY_MATRIX>

admin/index.php

@@ -226,7 +226,7 @@ if (!core_tables_exist()) {
     }
     if (empty($confirmrelease)) {
         require_once($CFG->libdir.'/environmentlib.php');
-        list($envstatus, $environment_results) = check_moodle_environment(normalize_version($release), ENV_SELECT_RELEASE);
+        list($envstatus, $environmentresults) = check_moodle_environment(normalize_version($release), ENV_SELECT_RELEASE);
         $strcurrentrelease = get_string('currentrelease');
 
         $PAGE->navbar->add($strcurrentrelease);
@@ -235,7 +235,7 @@ if (!core_tables_exist()) {
         $PAGE->set_cacheable(false);
 
         $output = $PAGE->get_renderer('core', 'admin');
-        echo $output->install_environment_page($maturity, $envstatus, $environment_results, $release);
+        echo $output->install_environment_page($maturity, $envstatus, $environmentresults, $release);
         die();
     }
 
@@ -358,9 +358,9 @@ if (!$cache and $version > $CFG->version) {  // upgrade
         echo $output->upgrade_confirm_page($a->newversion, $maturity, $testsite);
         die();
 
-    } else if (empty($confirmrelease)){
+    } else if (empty($confirmrelease)) {
         require_once($CFG->libdir.'/environmentlib.php');
-        list($envstatus, $environment_results) = check_moodle_environment($release, ENV_SELECT_RELEASE);
+        list($envstatus, $environmentresults) = check_moodle_environment($release, ENV_SELECT_RELEASE);
         $strcurrentrelease = get_string('currentrelease');
 
         $PAGE->navbar->add($strcurrentrelease);
@@ -368,7 +368,7 @@ if (!$cache and $version > $CFG->version) {  // upgrade
         $PAGE->set_heading($strcurrentrelease);
         $PAGE->set_cacheable(false);
 
-        echo $output->upgrade_environment_page($release, $envstatus, $environment_results);
+        echo $output->upgrade_environment_page($release, $envstatus, $environmentresults);
         die();
 
     } else if (empty($confirmplugins)) {
@@ -533,7 +533,10 @@ if (!$cache and $branch <> $CFG->branch) {  // Update the branch
 
 if (!$cache and moodle_needs_upgrading()) {
 
-    $PAGE->set_url(new moodle_url($PAGE->url, array('confirmplugincheck' => $confirmplugins)));
+    $PAGE->set_url(new moodle_url($PAGE->url, array(
+        'confirmrelease' => $confirmrelease,
+        'confirmplugincheck' => $confirmplugins,
+    )));
 
     check_upgrade_key($upgradekeyhash);
 
@@ -543,7 +546,21 @@ if (!$cache and moodle_needs_upgrading()) {
         $pluginman = core_plugin_manager::instance();
         $output = $PAGE->get_renderer('core', 'admin');
 
-        if (!$confirmplugins) {
+        if (empty($confirmrelease)) {
+            require_once($CFG->libdir . '/environmentlib.php');
+
+            list($envstatus, $environmentresults) = check_moodle_environment($release, ENV_SELECT_RELEASE);
+            $strcurrentrelease = get_string('currentrelease');
+
+            $PAGE->navbar->add($strcurrentrelease);
+            $PAGE->set_title($strcurrentrelease);
+            $PAGE->set_heading($strcurrentrelease);
+            $PAGE->set_cacheable(false);
+
+            echo $output->upgrade_environment_page($release, $envstatus, $environmentresults);
+            die();
+
+        } else if (!$confirmplugins) {
             $strplugincheck = get_string('plugincheck');
 
             $PAGE->navbar->add($strplugincheck);
@@ -802,7 +819,7 @@ $SESSION->admin_critical_warning = ($insecuredataroot==INSECURE_DATAROOT_ERROR);
 $adminroot = admin_get_root();
 
 // Check if there are any new admin settings which have still yet to be set
-if (any_new_admin_settings($adminroot)){
+if (any_new_admin_settings($adminroot)) {
     redirect('upgradesettings.php');
 }
 

admin/message.php

@@ -33,13 +33,17 @@ $allprocessors = get_message_processors();
 $processors = array_filter($allprocessors, function($processor) {
     return $processor->enabled;
 });
+$disabledprocessors = array_filter($allprocessors, function($processor) {
+    return !$processor->enabled;
+});
+
 // Fetch message providers.
 $providers = get_message_providers();
 // Fetch the manage message outputs interface.
 $preferences = get_message_output_default_preferences();
 
 if (($form = data_submitted()) && confirm_sesskey()) {
-    $preferences = array();
+    $newpreferences = array();
     // Prepare default message outputs settings.
     foreach ($providers as $provider) {
         $componentproviderbase = $provider->component.'_'.$provider->name;
@@ -47,9 +51,9 @@ if (($form = data_submitted()) && confirm_sesskey()) {
         $providerdisabled = false;
         if (!isset($form->$disableprovidersetting)) {
             $providerdisabled = true;
-            $preferences[$disableprovidersetting] = 1;
+            $newpreferences[$disableprovidersetting] = 1;
         } else {
-            $preferences[$disableprovidersetting] = 0;
+            $newpreferences[$disableprovidersetting] = 0;
         }
 
         foreach (array('permitted', 'loggedin', 'loggedoff') as $setting) {
@@ -75,20 +79,36 @@ if (($form = data_submitted()) && confirm_sesskey()) {
                         $form->{$componentproviderbase.'_loggedoff'}[$processor->name] = 1;
                     }
                     // Record the site preference.
-                    $preferences[$processor->name.'_provider_'.$componentprovidersetting] = $value;
+                    $newpreferences[$processor->name.'_provider_'.$componentprovidersetting] = $value;
                 }
-            } else if (array_key_exists($componentprovidersetting, $form)) {
-                // We must be processing loggedin or loggedoff checkboxes. Store
-                // defained comma-separated processors as setting value.
-                // Using array_filter eliminates elements set to 0 above.
-                $value = join(',', array_keys(array_filter($form->{$componentprovidersetting})));
+            } else {
+                $newsettings = array();
+                if (property_exists($form, $componentprovidersetting)) {
+                    // We must be processing loggedin or loggedoff checkboxes.
+                    // Store defained comma-separated processors as setting value.
+                    // Using array_filter eliminates elements set to 0 above.
+                    $newsettings = array_keys(array_filter($form->{$componentprovidersetting}));
+                }
+
+                // Let's join existing setting values for disabled processors.
+                $property = 'message_provider_'.$componentprovidersetting;
+                if (property_exists($preferences, $property)) {
+                    $existingsetting = $preferences->$property;
+                    foreach ($disabledprocessors as $disable) {
+                        if (strpos($existingsetting, $disable->name) > -1) {
+                            $newsettings[] = $disable->name;
+                        }
+                    }
+                }
+
+                $value = join(',', $newsettings);
                 if (empty($value)) {
                     $value = null;
                 }
             }
             if ($setting != 'permitted') {
                 // We have already recoded site preferences for 'permitted' type.
-                $preferences['message_provider_'.$componentprovidersetting] = $value;
+                $newpreferences['message_provider_'.$componentprovidersetting] = $value;
             }
         }
     }
@@ -102,7 +122,7 @@ if (($form = data_submitted()) && confirm_sesskey()) {
         \core_message\api::update_processor_status($processor, $enabled);
     }
 
-    foreach ($preferences as $name => $value) {
+    foreach ($newpreferences as $name => $value) {
         set_config($name, $value, 'message');
     }
     $transaction->allow_commit();

admin/registration/confirmregistration.php

@@ -45,8 +45,8 @@ $error = optional_param('error', '', PARAM_ALPHANUM);
 
 admin_externalpage_setup('registrationmoodleorg');
 
-if ($url !== HUB_MOODLEORGHUBURL) {
-    // Allow other plugins to confirm registration on hubs other than moodle.net . Plugins implementing this
+if (parse_url($url, PHP_URL_HOST) !== parse_url(HUB_MOODLEORGHUBURL, PHP_URL_HOST)) {
+    // Allow other plugins to confirm registration on custom hubs. Plugins implementing this
     // callback need to redirect or exit. See https://docs.moodle.org/en/Hub_registration .
     $callbacks = get_plugins_with_function('hub_registration');
     foreach ($callbacks as $plugintype => $plugins) {

admin/registration/forms.php

@@ -32,5 +32,4 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-debugging('Support for alternative hubs has been removed from Moodle in 3.4. For communication with moodle.net ' .
-    'see lib/classes/hub/ .', DEBUG_DEVELOPER);
+debugging('Support for alternative hubs has been removed from Moodle in 3.4.', DEBUG_DEVELOPER);

admin/registration/index.php

@@ -22,7 +22,7 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL
  * @copyright  (C) 1999 onwards Martin Dougiamas  http://dougiamas.com
  *
- * This page displays the site registration form for Moodle.net.
+ * This page displays the site registration form.
  * It handles redirection to the hub to continue the registration workflow process.
  * It also handles update operation by web service.
  */
@@ -48,7 +48,7 @@ if ($unregistration && \core\hub\registration::is_registered()) {
     }
 
     echo $OUTPUT->header();
-    echo $OUTPUT->heading(get_string('unregisterfrom', 'hub', 'Moodle.net'), 3, 'main');
+    echo $OUTPUT->heading(get_string('registerwithmoodleorgremove', 'core_hub'), 3, 'main');
     $siteunregistrationform->display();
     echo $OUTPUT->footer();
     exit;
@@ -82,7 +82,7 @@ if ($fromform = $siteregistrationform->get_data()) {
 
 echo $OUTPUT->header();
 
-// Current status of registration on Moodle.net.
+// Current status of registration.
 
 $notificationtype = \core\output\notification::NOTIFY_ERROR;
 if (\core\hub\registration::is_registered()) {
@@ -104,11 +104,11 @@ if (\core\hub\registration::is_registered()) {
 
 // Heading.
 if (\core\hub\registration::is_registered()) {
-    echo $OUTPUT->heading(get_string('updatesite', 'hub', 'Moodle.net'));
+    echo $OUTPUT->heading(get_string('registerwithmoodleorgupdate', 'core_hub'));
 } else if ($isinitialregistration) {
-    echo $OUTPUT->heading(get_string('completeregistration', 'hub'));
+    echo $OUTPUT->heading(get_string('registerwithmoodleorgcomplete', 'core_hub'));
 } else {
-    echo $OUTPUT->heading(get_string('registerwithmoodleorg', 'admin'));
+    echo $OUTPUT->heading(get_string('registerwithmoodleorg', 'core_hub'));
 }
 
 $renderer = $PAGE->get_renderer('core', 'admin');

admin/registration/renewregistration.php

@@ -39,8 +39,8 @@ $token = optional_param('token', '', PARAM_TEXT);
 
 admin_externalpage_setup('registrationmoodleorg');
 
-if ($url !== HUB_MOODLEORGHUBURL) {
-    // Allow other plugins to renew registration on hubs other than moodle.net . Plugins implementing this
+if (parse_url($url, PHP_URL_HOST) !== parse_url(HUB_MOODLEORGHUBURL, PHP_URL_HOST)) {
+    // Allow other plugins to renew registration on custom hubs. Plugins implementing this
     // callback need to redirect or exit. See https://docs.moodle.org/en/Hub_registration .
     $callbacks = get_plugins_with_function('hub_registration');
     foreach ($callbacks as $plugintype => $plugins) {
@@ -56,7 +56,7 @@ if ($url !== HUB_MOODLEORGHUBURL) {
 
 echo $OUTPUT->header();
 echo $OUTPUT->heading(get_string('renewregistration', 'hub'), 3, 'main');
-$hublink = html_writer::tag('a', 'Moodle.net', array('href' => HUB_MOODLEORGHUBURL));
+$hublink = html_writer::tag('a', HUB_MOODLEORGHUBURL, array('href' => HUB_MOODLEORGHUBURL));
 
 $deletedregmsg = get_string('previousregistrationdeleted', 'hub', $hublink);
 

admin/renderer.php

@@ -361,14 +361,15 @@ class core_admin_renderer extends plugin_renderer_base {
         $output = '';
 
         if ($checker->enabled()) {
-            $output .= $this->container_start('checkforupdates');
+            $output .= $this->container_start('checkforupdates mb-4');
             $output .= $this->single_button(
                 new moodle_url($reloadurl, array('fetchupdates' => 1)),
                 get_string('checkforupdates', 'core_plugin')
             );
             if ($timefetched = $checker->get_last_timefetched()) {
                 $timefetched = userdate($timefetched, get_string('strftimedatetime', 'core_langconfig'));
-                $output .= $this->container(get_string('checkforupdateslast', 'core_plugin', $timefetched), 'lasttimefetched');
+                $output .= $this->container(get_string('checkforupdateslast', 'core_plugin', $timefetched),
+                    'lasttimefetched small text-muted mt-1');
             }
             $output .= $this->container_end();
         }
@@ -823,7 +824,7 @@ class core_admin_renderer extends plugin_renderer_base {
             }
         }
 
-        $updateinfo .= $this->container_start('checkforupdates');
+        $updateinfo .= $this->container_start('checkforupdates mt-1');
         $fetchurl = new moodle_url('/admin/index.php', array('fetchupdates' => 1, 'sesskey' => sesskey(), 'cache' => 0));
         $updateinfo .= $this->single_button($fetchurl, get_string('checkforupdates', 'core_plugin'));
         if ($fetch) {
@@ -912,7 +913,7 @@ class core_admin_renderer extends plugin_renderer_base {
      */
     protected function moodle_available_update_info(\core\update\info $updateinfo) {
 
-        $boxclasses = 'moodleupdateinfo';
+        $boxclasses = 'moodleupdateinfo mb-2';
         $info = array();
 
         if (isset($updateinfo->release)) {
@@ -932,7 +933,8 @@ class core_admin_renderer extends plugin_renderer_base {
         }
 
         if (isset($updateinfo->download)) {
-            $info[] = html_writer::link($updateinfo->download, get_string('download'), array('class' => 'info download'));
+            $info[] = html_writer::link($updateinfo->download, get_string('download'),
+                array('class' => 'info download btn btn-secondary'));
         }
 
         if (isset($updateinfo->url)) {
@@ -940,9 +942,9 @@ class core_admin_renderer extends plugin_renderer_base {
                 array('class' => 'info more'));
         }
 
-        $box  = $this->output->box_start($boxclasses);
-        $box .= $this->output->box(implode(html_writer::tag('span', ' ', array('class' => 'separator')), $info), '');
-        $box .= $this->output->box_end();
+        $box  = $this->output->container_start($boxclasses);
+        $box .= $this->output->container(implode(html_writer::tag('span', ' | ', array('class' => 'separator')), $info), '');
+        $box .= $this->output->container_end();
 
         return $box;
     }
@@ -1054,7 +1056,7 @@ class core_admin_renderer extends plugin_renderer_base {
                 $displayname = new html_table_cell(
                     $icon.
                     html_writer::span($plugin->displayname, 'pluginname').
-                    html_writer::div($plugin->get_dir(), 'plugindir')
+                    html_writer::div($plugin->get_dir(), 'plugindir text-muted small')
                 );
 
                 $versiondb = new html_table_cell($plugin->versiondb);
@@ -1089,7 +1091,7 @@ class core_admin_renderer extends plugin_renderer_base {
                         break;
                     case core_plugin_manager::PLUGIN_STATUS_NODB:
                     case core_plugin_manager::PLUGIN_STATUS_UPTODATE:
-                        $statusclass .= $dependenciesok ? '' : 'badge-warning';
+                        $statusclass .= $dependenciesok ? 'badge-light' : 'badge-warning';
                         break;
                 }
                 $status = html_writer::span(get_string('status_' . $statuscode, 'core_plugin'), $statusclass);
@@ -1099,7 +1101,7 @@ class core_admin_renderer extends plugin_renderer_base {
                         new moodle_url($this->page->url, array('abortinstall' => $plugin->component)),
                         get_string('cancelinstallone', 'core_plugin'),
                         'post',
-                        array('class' => 'actionbutton cancelinstallone')
+                        array('class' => 'actionbutton cancelinstallone d-block mt-1')
                     );
                 }
 
@@ -1108,7 +1110,7 @@ class core_admin_renderer extends plugin_renderer_base {
                         new moodle_url($this->page->url, array('abortupgrade' => $plugin->component)),
                         get_string('cancelupgradeone', 'core_plugin'),
                         'post',
-                        array('class' => 'actionbutton cancelupgradeone')
+                        array('class' => 'actionbutton cancelupgradeone d-block mt-1')
                     );
                 }
 
@@ -1178,7 +1180,7 @@ class core_admin_renderer extends plugin_renderer_base {
             }
         }
 
-        $out .= $this->output->container_start('actions');
+        $out .= $this->output->container_start('actions mb-2');
 
         $installableupdates = $pluginman->filter_installable($pluginman->available_updates());
         if ($installableupdates) {
@@ -1186,7 +1188,7 @@ class core_admin_renderer extends plugin_renderer_base {
                 new moodle_url($this->page->url, array('installupdatex' => 1)),
                 get_string('updateavailableinstallall', 'core_admin', count($installableupdates)),
                 'post',
-                array('class' => 'singlebutton updateavailableinstallall')
+                array('class' => 'singlebutton updateavailableinstallall mr-1')
             );
         }
 
@@ -1195,7 +1197,7 @@ class core_admin_renderer extends plugin_renderer_base {
                 new moodle_url($this->page->url, array('abortinstallx' => 1)),
                 get_string('cancelinstallall', 'core_plugin', count($installabortable)),
                 'post',
-                array('class' => 'singlebutton cancelinstallall')
+                array('class' => 'singlebutton cancelinstallall mr-1')
             );
         }
 
@@ -1204,15 +1206,17 @@ class core_admin_renderer extends plugin_renderer_base {
                 new moodle_url($this->page->url, array('abortupgradex' => 1)),
                 get_string('cancelupgradeall', 'core_plugin', count($upgradeabortable)),
                 'post',
-                array('class' => 'singlebutton cancelupgradeall')
+                array('class' => 'singlebutton cancelupgradeall mr-1')
             );
         }
 
         $out .= html_writer::div(html_writer::link(new moodle_url($this->page->url, array('showallplugins' => 0)),
-            get_string('plugincheckattention', 'core_plugin')).' '.html_writer::span($sumattention, 'badge'));
+            get_string('plugincheckattention', 'core_plugin')).' '.html_writer::span($sumattention, 'badge badge-light'),
+            'btn btn-link mr-1');
 
         $out .= html_writer::div(html_writer::link(new moodle_url($this->page->url, array('showallplugins' => 1)),
-            get_string('plugincheckall', 'core_plugin')).' '.html_writer::span($sumtotal, 'badge'));
+            get_string('plugincheckall', 'core_plugin')).' '.html_writer::span($sumtotal, 'badge badge-light'),
+            'btn btn-link mr-1');
 
         $out .= $this->output->container_end(); // End of .actions container.
         $out .= $this->output->container_end(); // End of #plugins-check-info container.
@@ -1280,12 +1284,13 @@ class core_admin_renderer extends plugin_renderer_base {
             }
         }
 
-        $out  = $this->output->container_start('plugins-check-dependencies');
+        $out  = $this->output->container_start('plugins-check-dependencies mb-4');
 
         if ($unavailable or $unknown) {
             $out .= $this->output->heading(get_string('misdepsunavail', 'core_plugin'));
             if ($unknown) {
-                $out .= $this->output->notification(get_string('misdepsunknownlist', 'core_plugin', implode($unknown, ', ')));
+                $out .= $this->output->render((new \core\output\notification(get_string('misdepsunknownlist', 'core_plugin',
+                    implode(', ', $unknown))))->set_show_closebutton(false));
             }
             if ($unavailable) {
                 $unavailablelist = array();
@@ -1299,18 +1304,18 @@ class core_admin_renderer extends plugin_renderer_base {
                     }
                     $unavailablelist[] = $unavailablelistitem;
                 }
-                $out .= $this->output->notification(get_string('misdepsunavaillist', 'core_plugin',
-                    implode($unavailablelist, ', ')));
+                $out .= $this->output->render((new \core\output\notification(get_string('misdepsunavaillist', 'core_plugin',
+                    implode(', ', $unavailablelist))))->set_show_closebutton(false));
             }
-            $out .= $this->output->container_start('plugins-check-dependencies-actions');
+            $out .= $this->output->container_start('plugins-check-dependencies-actions mb-4');
             $out .= ' '.html_writer::link(new moodle_url('/admin/tool/installaddon/'),
-                get_string('dependencyuploadmissing', 'core_plugin'));
+                get_string('dependencyuploadmissing', 'core_plugin'), array('class' => 'btn btn-secondary'));
             $out .= $this->output->container_end(); // End of .plugins-check-dependencies-actions container.
         }
 
         if ($available) {
             $out .= $this->output->heading(get_string('misdepsavail', 'core_plugin'));
-            $out .= $this->output->container_start('plugins-check-dependencies-actions');
+            $out .= $this->output->container_start('plugins-check-dependencies-actions mb-2');
 
             $installable = $pluginman->filter_installable($available);
             if ($installable) {
@@ -1318,12 +1323,13 @@ class core_admin_renderer extends plugin_renderer_base {
                     new moodle_url($this->page->url, array('installdepx' => 1)),
                     get_string('dependencyinstallmissing', 'core_plugin', count($installable)),
                     'post',
-                    array('class' => 'singlebutton dependencyinstallmissing')
+                    array('class' => 'singlebutton dependencyinstallmissing d-inline-block mr-1')
                 );
             }
 
             $out .= html_writer::div(html_writer::link(new moodle_url('/admin/tool/installaddon/'),
-                get_string('dependencyuploadmissing', 'core_plugin')), 'dependencyuploadmissing');
+                get_string('dependencyuploadmissing', 'core_plugin'), array('class' => 'btn btn-link')),
+                'dependencyuploadmissing d-inline-block mr-1');
 
             $out .= $this->output->container_end(); // End of .plugins-check-dependencies-actions container.
 
@@ -1364,7 +1370,7 @@ class core_admin_renderer extends plugin_renderer_base {
                 if ($CFG->branch == str_replace('.', '', $moodle->release)) {
                     $supportedmoodles[] = html_writer::span($moodle->release, 'badge badge-success');
                 } else {
-                    $supportedmoodles[] = html_writer::span($moodle->release, 'label');
+                    $supportedmoodles[] = html_writer::span($moodle->release, 'badge badge-light');
                 }
             }
 
@@ -1378,7 +1384,7 @@ class core_admin_renderer extends plugin_renderer_base {
                 }
                 $info = html_writer::div(
                     get_string('requiredby', 'core_plugin', implode(', ', $requriedby)),
-                    'requiredby'
+                    'requiredby mb-1'
                 );
             } else {
                 $info = '';
@@ -1389,35 +1395,36 @@ class core_admin_renderer extends plugin_renderer_base {
             $info .= html_writer::div(
                 html_writer::link('https://moodle.org/plugins/view.php?plugin='.$plugin->component,
                     get_string('misdepinfoplugin', 'core_plugin')),
-                'misdepinfoplugin'
+                'misdepinfoplugin d-inline-block mr-3 mb-1'
             );
 
             $info .= html_writer::div(
                 html_writer::link('https://moodle.org/plugins/pluginversion.php?id='.$plugin->version->id,
                     get_string('misdepinfoversion', 'core_plugin')),
-                'misdepinfoversion'
+                'misdepinfoversion d-inline-block mr-3 mb-1'
             );
 
-            $info .= html_writer::div(html_writer::link($plugin->version->downloadurl, get_string('download')), 'misdepdownload');
+            $info .= html_writer::div(html_writer::link($plugin->version->downloadurl, get_string('download')),
+                'misdepdownload d-inline-block mr-3 mb-1');
 
             if ($pluginman->is_remote_plugin_installable($plugin->component, $plugin->version->version, $reason)) {
                 $info .= $this->output->single_button(
                     new moodle_url($this->page->url, array('installdep' => $plugin->component)),
                     get_string('dependencyinstall', 'core_plugin'),
                     'post',
-                    array('class' => 'singlebutton dependencyinstall')
+                    array('class' => 'singlebutton dependencyinstall mr-3 mb-1')
                 );
             } else {
                 $reasonhelp = $this->info_remote_plugin_not_installable($reason);
                 if ($reasonhelp) {
-                    $info .= html_writer::div($reasonhelp, 'reasonhelp dependencyinstall');
+                    $info .= html_writer::div($reasonhelp, 'reasonhelp dependencyinstall d-inline-block mr-3 mb-1');
                 }
             }
 
             $info .= $this->output->container_end(); // End of .actions container.
 
             $table->data[] = array(
-                html_writer::div($plugin->name, 'name').' '.html_writer::div($plugin->component, 'component'),
+                html_writer::div($plugin->name, 'name').' '.html_writer::div($plugin->component, 'component text-muted small'),
                 $plugin->version->release,
                 $plugin->version->version,
                 implode($supportedmoodles, ' '),
@@ -1463,22 +1470,24 @@ class core_admin_renderer extends plugin_renderer_base {
         foreach ($pluginman->resolve_requirements($plugin, $version) as $reqname => $reqinfo) {
             if ($reqname === 'core') {
                 if ($reqinfo->status == $pluginman::REQUIREMENT_STATUS_OK) {
-                    $class = 'requires-ok';
+                    $class = 'requires-ok text-muted';
                     $label = '';
                 } else {
                     $class = 'requires-failed';
                     $label = html_writer::span(get_string('dependencyfails', 'core_plugin'), 'badge badge-danger');
                 }
-                $requires[] = html_writer::tag('li',
-                    html_writer::span(get_string('moodleversion', 'core_plugin', $plugin->versionrequires), 'dep dep-core').
-                    ' '.$label, array('class' => $class));
+                if ($reqinfo->reqver != ANY_VERSION) {
+                    $requires[] = html_writer::tag('li',
+                        html_writer::span(get_string('moodleversion', 'core_plugin', $plugin->versionrequires), 'dep dep-core').
+                        ' '.$label, array('class' => $class));
+                }
 
             } else {
                 $actions = array();
 
                 if ($reqinfo->status == $pluginman::REQUIREMENT_STATUS_OK) {
                     $label = '';
-                    $class = 'requires-ok';
+                    $class = 'requires-ok text-muted';
 
                 } else if ($reqinfo->status == $pluginman::REQUIREMENT_STATUS_MISSING) {
                     if ($reqinfo->availability == $pluginman::REQUIREMENT_AVAILABLE) {
@@ -1532,13 +1541,14 @@ class core_admin_renderer extends plugin_renderer_base {
             return '';
         }
 
-        $out = html_writer::tag('ul', implode("\n", $requires));
+        $out = html_writer::tag('ul', implode("\n", $requires), array('class' => 'm-0'));
 
         if ($displayuploadlink) {
             $out .= html_writer::div(
                 html_writer::link(
                     new moodle_url('/admin/tool/installaddon/'),
-                    get_string('dependencyuploadmissing', 'core_plugin')
+                    get_string('dependencyuploadmissing', 'core_plugin'),
+                    array('class' => 'btn btn-secondary btn-sm m-1')
                 ),
                 'dependencyuploadmissing'
             );
@@ -1548,7 +1558,8 @@ class core_admin_renderer extends plugin_renderer_base {
             $out .= html_writer::div(
                 html_writer::link(
                     new moodle_url($this->page->url, array('sesskey' => sesskey(), 'fetchupdates' => 1)),
-                    get_string('checkforupdates', 'core_plugin')
+                    get_string('checkforupdates', 'core_plugin'),
+                    array('class' => 'btn btn-secondary btn-sm m-1')
                 ),
                 'checkforupdates'
             );
@@ -2110,6 +2121,27 @@ class core_admin_renderer extends plugin_renderer_base {
      * @return string
      */
     public function moodleorg_registration_message() {
-        return format_text(get_string('registermoodlenet', 'admin'), FORMAT_HTML, ['noclean' => true]);
+
+        $out = format_text(get_string('registerwithmoodleorginfo', 'core_hub'), FORMAT_MARKDOWN);
+
+        $out .= html_writer::link(
+            new moodle_url('/admin/settings.php', ['section' => 'moodleservices']),
+            $this->output->pix_icon('i/info', '').' '.get_string('registerwithmoodleorginfoapp', 'core_hub'),
+            ['class' => 'btn btn-link', 'role' => 'opener', 'target' => '_href']
+        );
+
+        $out .= html_writer::link(
+            HUB_MOODLEORGHUBURL,
+            $this->output->pix_icon('i/stats', '').' '.get_string('registerwithmoodleorginfostats', 'core_hub'),
+            ['class' => 'btn btn-link', 'role' => 'opener', 'target' => '_href']
+        );
+
+        $out .= html_writer::link(
+            HUB_MOODLEORGHUBURL.'/sites',
+            $this->output->pix_icon('i/location', '').' '.get_string('registerwithmoodleorginfosites', 'core_hub'),
+            ['class' => 'btn btn-link', 'role' => 'opener', 'target' => '_href']
+        );
+
+        return $this->output->box($out);
     }
 }

admin/roles/admins.php

@@ -36,15 +36,12 @@ if (!is_siteadmin()) {
 }
 
 $admisselector = new core_role_admins_existing_selector();
-$admisselector->set_extra_fields(array('username', 'email'));
-
 $potentialadmisselector = new core_role_admins_potential_selector();
-$potentialadmisselector->set_extra_fields(array('username', 'email'));
 
 if (optional_param('add', false, PARAM_BOOL) and confirm_sesskey()) {
     if ($userstoadd = $potentialadmisselector->get_selected_users()) {
         $user = reset($userstoadd);
-        $username = fullname($user) . " ($user->username, $user->email)";
+        $username = $potentialadmisselector->output_user($user);
         echo $OUTPUT->header();
         $yesurl = new moodle_url('/admin/roles/admins.php', array('confirmadd'=>$user->id, 'sesskey'=>sesskey()));
         echo $OUTPUT->confirm(get_string('confirmaddadmin', 'core_role', $username), $yesurl, $PAGE->url);
@@ -58,7 +55,7 @@ if (optional_param('add', false, PARAM_BOOL) and confirm_sesskey()) {
         if ($USER->id == $user->id) {
             // Can not remove self.
         } else {
-            $username = fullname($user) . " ($user->username, $user->email)";
+            $username = $admisselector->output_user($user);
             echo $OUTPUT->header();
             $yesurl = new moodle_url('/admin/roles/admins.php', array('confirmdel'=>$user->id, 'sesskey'=>sesskey()));
             echo $OUTPUT->confirm(get_string('confirmdeladmin', 'core_role', $username), $yesurl, $PAGE->url);

admin/roles/classes/define_role_table_advanced.php

@@ -157,19 +157,19 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi
         // Allowed roles.
         $allow = optional_param_array('allowassign', null, PARAM_INT);
         if (!is_null($allow)) {
-            $this->allowassign = $allow;
+            $this->allowassign = array_filter($allow);
         }
         $allow = optional_param_array('allowoverride', null, PARAM_INT);
         if (!is_null($allow)) {
-            $this->allowoverride = $allow;
+            $this->allowoverride = array_filter($allow);
         }
         $allow = optional_param_array('allowswitch', null, PARAM_INT);
         if (!is_null($allow)) {
-            $this->allowswitch = $allow;
+            $this->allowswitch = array_filter($allow);
         }
         $allow = optional_param_array('allowview', null, PARAM_INT);
         if (!is_null($allow)) {
-            $this->allowview = $allow;
+            $this->allowview = array_filter($allow);
         }
 
         // Now read the permissions for each capability.
@@ -589,7 +589,9 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi
         if ($this->roleid == 0) {
             $options[-1] = get_string('thisnewrole', 'core_role');
         }
-        return html_writer::select($options, 'allow'.$type.'[]', $selected, false, array('multiple' => 'multiple',
+        return
+            html_writer::empty_tag('input', array('type' => 'hidden', 'name' => 'allow'.$type.'[]', 'value' => "")) .
+            html_writer::select($options, 'allow'.$type.'[]', $selected, false, array('multiple' => 'multiple',
             'size' => 10, 'class' => 'form-control'));
     }
 

admin/settings/badges.php

@@ -105,13 +105,6 @@ if (($hassiteconfig || has_any_capability(array(
             new lang_string('sitebackpack_help', 'badges'),
             1, $choices));
 
-    $warning = badges_verify_site_backpack();
-    if (!empty($warning)) {
-        $backpacksettings->add(new admin_setting_description('badges_site_backpack_verify',
-            new lang_string('sitebackpackverify', 'badges'),
-            $warning));
-    }
-
     $ADMIN->add('badges', $backpacksettings);
 
     $ADMIN->add('badges',

admin/settings/server.php

@@ -97,7 +97,10 @@ $options = array(
     GETREMOTEADDR_SKIP_HTTP_CLIENT_IP => 'HTTP_X_FORWARDED_FOR, REMOTE_ADDR',
     GETREMOTEADDR_SKIP_HTTP_X_FORWARDED_FOR => 'HTTP_CLIENT, REMOTE_ADDR',
     GETREMOTEADDR_SKIP_HTTP_X_FORWARDED_FOR|GETREMOTEADDR_SKIP_HTTP_CLIENT_IP => 'REMOTE_ADDR');
-$temp->add(new admin_setting_configselect('getremoteaddrconf', new lang_string('getremoteaddrconf', 'admin'), new lang_string('configgetremoteaddrconf', 'admin'), 0, $options));
+$temp->add(new admin_setting_configselect('getremoteaddrconf', new lang_string('getremoteaddrconf', 'admin'),
+    new lang_string('configgetremoteaddrconf', 'admin'),
+    GETREMOTEADDR_SKIP_HTTP_X_FORWARDED_FOR|GETREMOTEADDR_SKIP_HTTP_CLIENT_IP, $options));
+$temp->add(new admin_setting_configtext('reverseproxyignore', new lang_string('reverseproxyignore', 'admin'), new lang_string('configreverseproxyignore', 'admin'), ''));
 
 $temp->add(new admin_setting_heading('webproxy', new lang_string('webproxy', 'admin'), new lang_string('webproxyinfo', 'admin')));
 $temp->add(new admin_setting_configtext('proxyhost', new lang_string('proxyhost', 'admin'), new lang_string('configproxyhost', 'admin'), '', PARAM_HOST));

admin/tasklogs.php

@@ -49,6 +49,8 @@ $logid = optional_param('logid', null, PARAM_INT);
 $download = optional_param('download', false, PARAM_BOOL);
 
 if (null !== $logid) {
+    // Raise memory limit in case the log is large.
+    raise_memory_limit(MEMORY_HUGE);
     $log = $DB->get_record('task_log', ['id' => $logid], '*', MUST_EXIST);
 
     if ($download) {
@@ -65,7 +67,7 @@ $renderer = $PAGE->get_renderer('tool_task');
 echo $OUTPUT->header();
 echo $OUTPUT->render_from_template('core_admin/tasklogs', (object) [
     'action' => $pageurl->out(),
-    'filter' => $filter,
+    'filter' => htmlentities($filter),
     'resultfilteroptions' => [
         (object) [
             'value' => -1,

admin/testoutgoingmailconf.php

@@ -46,13 +46,14 @@ if ($data) {
     $emailuser->email = $data->recipient;
     $emailuser->id = -99;
 
-    $subject = get_string('testoutgoingmailconf_subject', 'admin', $SITE->fullname);
+    $subject = get_string('testoutgoingmailconf_subject', 'admin',
+        format_string($SITE->fullname, true, ['context' => context_system::instance()]));
     $messagetext = get_string('testoutgoingmailconf_message', 'admin');
 
     // Manage Moodle debugging options.
     $debuglevel = $CFG->debug;
     $debugdisplay = $CFG->debugdisplay;
-    $debugsmtp = $CFG->debugsmtp;
+    $debugsmtp = $CFG->debugsmtp ?? null; // This might not be set as it's optional.
     $CFG->debugdisplay = true;
     $CFG->debugsmtp = true;
     $CFG->debug = 15;
@@ -66,7 +67,12 @@ if ($data) {
     // Restore Moodle debugging options.
     $CFG->debug = $debuglevel;
     $CFG->debugdisplay = $debugdisplay;
-    $CFG->debugsmtp = $debugsmtp;
+
+    // Restore the debugsmtp config, if it was set originally.
+    unset($CFG->debugsmtp);
+    if (!is_null($debugsmtp)) {
+        $CFG->debugsmtp = $debugsmtp;
+    }
 
     if ($success) {
         $msgparams = new stdClass();

admin/tests/behat/behat_admin.php

@@ -48,13 +48,11 @@ class behat_admin extends behat_base {
      * @param TableNode $table
      */
     public function i_set_the_following_administration_settings_values(TableNode $table) {
-
         if (!$data = $table->getRowsHash()) {
             return;
         }
 
         foreach ($data as $label => $value) {
-
             $this->execute('behat_navigation::i_select_from_flat_navigation_drawer', [get_string('administrationsite')]);
 
             // Search by label.
@@ -76,38 +74,17 @@ class behat_admin extends behat_base {
                         "@id=//span[contains(normalize-space(.), $label)]/preceding-sibling::label[1]/@for]";
                 $fieldnode = $this->find('xpath', $fieldxpath, $exception);
 
-                $formfieldtypenode = $this->find('xpath', $fieldxpath .
-                        "/ancestor::div[contains(concat(' ', @class, ' '), ' form-setting ')]" .
-                        "/child::div[contains(concat(' ', @class, ' '),  ' form-')]/child::*/parent::div");
-
             } catch (ElementNotFoundException $e) {
-
                 // Multi element settings, interacting only the first one.
                 $fieldxpath = "//*[label[contains(., $label)]|span[contains(., $label)]]" .
                         "/ancestor::div[contains(concat(' ', normalize-space(@class), ' '), ' form-item ')]" .
                         "/descendant::div[contains(concat(' ', @class, ' '), ' form-group ')]" .
                         "/descendant::*[self::input | self::textarea | self::select]" .
                         "[not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')]";
-                $fieldnode = $this->find('xpath', $fieldxpath);
-
-                // It is the same one that contains the type.
-                $formfieldtypenode = $fieldnode;
             }
 
-            // Getting the class which contains the field type.
-            $classes = explode(' ', $formfieldtypenode->getAttribute('class'));
-            $type = false;
-            foreach ($classes as $class) {
-                if (substr($class, 0, 5) == 'form-') {
-                    $type = substr($class, 5);
-                }
-            }
-
-            // Instantiating the appropiate field type.
-            $field = behat_field_manager::get_field_instance($type, $fieldnode, $this->getSession());
-            $field->set_value($value);
-
-            $this->find_button(get_string('savechanges'))->press();
+            $this->execute('behat_forms::i_set_the_field_with_xpath_to', [$fieldxpath, $value]);
+            $this->execute("behat_general::i_click_on", [get_string('savechanges'), 'button']);
         }
     }
 

admin/tool/analytics/classes/task/predict_models.php

@@ -59,6 +59,9 @@ class predict_models extends \core\task\scheduled_task {
         }
 
         foreach ($models as $model) {
+
+            $renderer = $PAGE->get_renderer('tool_analytics');
+
             $result = $model->predict();
 
             // Reset the page as some indicators may call external functions that overwrite the page context.
@@ -66,7 +69,6 @@ class predict_models extends \core\task\scheduled_task {
 
             if ($result) {
                 echo $OUTPUT->heading(get_string('modelresults', 'tool_analytics', $model->get_name()));
-                $renderer = $PAGE->get_renderer('tool_analytics');
                 echo $renderer->render_get_predictions_results(false, array(), $result, $model->get_analyser()->get_logs());
             }
         }

admin/tool/analytics/classes/task/train_models.php

@@ -70,6 +70,8 @@ class train_models extends \core\task\scheduled_task {
                 continue;
             }
 
+            $renderer = $PAGE->get_renderer('tool_analytics');
+
             $result = $model->train();
 
             // Reset the page as some indicators may call external functions that overwrite the page context.
@@ -77,8 +79,6 @@ class train_models extends \core\task\scheduled_task {
 
             if ($result) {
                 echo $OUTPUT->heading(get_string('modelresults', 'tool_analytics', $model->get_name()));
-
-                $renderer = $PAGE->get_renderer('tool_analytics');
                 echo $renderer->render_get_predictions_results($result, $model->get_analyser()->get_logs());
             }
         }

admin/tool/analytics/cli/evaluate_model.php

@@ -102,6 +102,8 @@ if ($options['reuse-prev-analysed']) {
     mtrace(get_string('evaluationinbatches', 'tool_analytics'));
 }
 
+$renderer = $PAGE->get_renderer('tool_analytics');
+
 $analyseroptions = array(
     'filter' => $options['filter'],
     'timesplitting' => $options['timesplitting'],
@@ -114,7 +116,6 @@ $results = $model->evaluate($analyseroptions);
 // Reset the page as some indicators may call external functions that overwrite the page context.
 \tool_analytics\output\helper::reset_page();
 
-$renderer = $PAGE->get_renderer('tool_analytics');
 echo $renderer->render_evaluate_results($results, $model->get_analyser()->get_logs());
 
 // Check that we have, at leasa,t 1 valid dataset (not necessarily good) to use.

admin/tool/behat/cli/util_single_run.php

@@ -164,6 +164,8 @@ if ($options['install']) {
         mtrace("Acceptance tests site installed");
     }
 
+    // Note: Do not build the themes here. This is done during the 'enable' stage.
+
 } else if ($options['drop']) {
     // Ensure no tests are running.
     test_lock::acquire('behat');
@@ -182,6 +184,10 @@ if ($options['install']) {
     // Enable test mode.
     behat_util::start_test_mode($options['add-core-features-to-theme'], $options['optimize-runs'], $parallel, $run);
 
+    // Themes are only built in the 'enable' command.
+    behat_util::build_themes();
+    mtrace("Testing environment themes built");
+
     // This is only displayed once for parallel install.
     if (empty($run)) {
         // Notify user that 2.5 profile has been converted to 3.5.

admin/tool/behat/index.php

@@ -32,7 +32,7 @@ require_once($CFG->libdir . '/behat/classes/behat_config_manager.php');
 // systems, but let's allow room for expansion.
 core_php_time_limit::raise(300);
 
-$filter = optional_param('filter', '', PARAM_ALPHANUMEXT);
+$filter = optional_param('filter', '', PARAM_NOTAGS);
 $type = optional_param('type', false, PARAM_ALPHAEXT);
 $component = optional_param('component', '', PARAM_ALPHAEXT);
 

admin/tool/behat/locallib.php

@@ -55,13 +55,13 @@ class tool_behat {
         // The loaded steps depends on the component specified.
         behat_config_manager::update_config_file($component, false);
 
-        // The Moodle\BehatExtension\HelpPrinter\MoodleDefinitionsPrinter will parse this search format.
+        // The Moodle\BehatExtension\Definition\Printer\ConsoleDefinitionInformationPrinter will parse this search format.
         if ($type) {
             $filter .= '&&' . $type;
         }
 
         if ($filter) {
-            $filteroption = ' -d "' . $filter . '"';
+            $filteroption = ' -d ' . escapeshellarg($filter);
         } else {
             $filteroption = ' -di';
         }

admin/tool/behat/renderer.php

@@ -44,13 +44,9 @@ class tool_behat_renderer extends plugin_renderer_base {
         global $CFG;
         require_once($CFG->libdir . '/behat/classes/behat_selectors.php');
 
-        $html = $this->generic_info();
-
-        // Form.
-        ob_start();
-        $form->display();
-        $html .= ob_get_contents();
-        ob_end_clean();
+        $html = $this->output->header();
+        $html .= $this->output->heading(get_string('pluginname', 'tool_behat'));
+        $html .= $form->render();
 
         if (empty($stepsdefinitions)) {
             $stepsdefinitions = get_string('nostepsdefinitions', 'tool_behat');
@@ -128,7 +124,9 @@ class tool_behat_renderer extends plugin_renderer_base {
      */
     public function render_error($msg) {
 
-        $html = $this->generic_info();
+        $html = $this->output->header();
+        $html .= $this->output->heading(get_string('pluginname', 'tool_behat'));
+        $html .= $this->generic_info();
 
         $a = new stdClass();
         $a->errormsg = $msg;
@@ -153,13 +151,7 @@ class tool_behat_renderer extends plugin_renderer_base {
      *
      * @return string
      */
-    protected function generic_info() {
-
-        $title = get_string('pluginname', 'tool_behat');
-
-        // Header.
-        $html = $this->output->header();
-        $html .= $this->output->heading($title);
+    public function generic_info() {
 
         // Info.
         $installurl = behat_command::DOCS_URL;
@@ -175,8 +167,7 @@ class tool_behat_renderer extends plugin_renderer_base {
         );
 
         // List of steps.
-        $html .= $this->output->box_start();
-        $html .= html_writer::tag('h3', get_string('infoheading', 'tool_behat'));
+        $html = $this->output->box_start();
         $html .= html_writer::tag('div', get_string('aim', 'tool_behat'));
         $html .= html_writer::start_tag('div');
         $html .= html_writer::start_tag('ul');

admin/tool/behat/steps_definitions_form.php

@@ -40,8 +40,14 @@ class steps_definitions_form extends moodleform {
      * @return void
      */
     public function definition() {
+        global $PAGE;
 
         $mform = $this->_form;
+        $output = $PAGE->get_renderer('tool_behat');
+
+        $mform->addElement('header', 'info', get_string('infoheading', 'tool_behat'));
+        $mform->setExpanded('info', false);
+        $mform->addElement('html', $output->generic_info());
 
         $mform->addElement('header', 'filters', get_string('stepsdefinitionsfilters', 'tool_behat'));
 

admin/tool/behat/styles.css

@@ -1,25 +1,31 @@
-.steps-definitions {
-    border-style: solid;
-    border-width: 1px;
-    border-color: #bbb;
-    padding: 5px;
-    margin: auto;
-    width: 50%;
+#page-admin-tool-behat-index .steps-definitions {
+    margin: 1rem auto;
 }
 
-.steps-definitions .step {
-    margin: 10px 0 10px 0;
+#page-admin-tool-behat-index .steps-definitions .step {
+    margin: 1rem 0 0 0;
+    border: 1px solid #eee;
+    padding: 1rem;
 }
 
-.steps-definitions .stepdescription {
-    color: #bf8c12;
+#page-admin-tool-behat-index .steps-definitions .stepdescription {
+    font-style: italic;
 }
 
-.steps-definitions .steptype {
+#page-admin-tool-behat-index .steps-definitions .stepcontent {
+    margin: 1rem 0;
+}
+
+#page-admin-tool-behat-index .steps-definitions .steptype {
     color: #1467a6;
-    margin-right: 5px;
+    margin-right: 1ex;
 }
 
-.steps-definitions .stepregex {
+#page-admin-tool-behat-index .steps-definitions .stepapipath {
+    font-family: monospace;
+    font-size: smaller;
+}
+
+#page-admin-tool-behat-index .steps-definitions .stepregex {
     color: #060;
 }

admin/tool/behat/tests/behat/get_and_set_fields_in_containers.feature

@@ -0,0 +1,27 @@
+@tool_behat
+Feature: Behat steps for interacting with form work
+  In order to test my Moodle code
+  As a developer
+  I need the Behat steps for form elements to work reliably
+
+  @javascript
+  Scenario: Test fields in containers
+    Given the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    When I log in as "admin"
+    And I am on "Course 1" course homepage
+    # Just get to any form.
+    And I navigate to "Edit settings" in current page administration
+    And I set the field "Course full name" in the "General" "fieldset" to "Frog"
+    And I set the following fields in the "Appearance" "fieldset" to these values:
+      | Show activity reports   | Yes |
+      | Number of announcements | 1   |
+    Then the field "Show activity reports" in the "Appearance" "fieldset" matches value "Yes"
+    And the field "Show activity reports" in the "Appearance" "fieldset" does not match value "No"
+    And the following fields in the "region-main" "region" match these values:
+      | Course full name        | Frog |
+      | Number of announcements | 1    |
+    And the following fields in the "region-main" "region" do not match these values:
+      | Course full name        | Course 1 |
+      | Number of announcements | 5        |

admin/tool/behat/tests/behat/list_steps.feature

@@ -26,3 +26,11 @@ Feature: List the system steps definitions
     Given I set the field "Contains" to "homepage"
     When I press "Filter"
     Then I should see "Opens Moodle homepage."
+
+  @javascript
+  Scenario: Filtering by the multiple words pattern
+    Given I set the field "Contains" to "should exist"
+    When I press "Filter"
+    Then I should not see "There aren't steps definitions matching this filter"
+    And I should see "Checks the provided element and selector type exists in the current page."
+    And I should see "Checks that an element and selector type exists in another element and selector type on the current page."

admin/tool/capability/renderer.php

@@ -75,6 +75,7 @@ class tool_capability_renderer extends plugin_renderer_base {
      * @return string
      */
     public function capability_comparison_table(array $capabilities, $contextid, array $roles) {
+        static $capabilitycontexts = array();
 
         $strpermissions = $this->get_permission_strings();
         $permissionclasses = $this->get_permission_classes();
@@ -92,8 +93,13 @@ class tool_capability_renderer extends plugin_renderer_base {
         }
         $table->data = array();
 
+        $childcontextsids = [];
         foreach ($capabilities as $capability) {
-            $contexts = tool_capability_calculate_role_data($capability, $roles);
+            if (empty($capabilitycontexts[$capability])) {
+                $capabilitycontexts[$capability] = tool_capability_calculate_role_data($capability, $roles);
+            }
+            $contexts = $capabilitycontexts[$capability];
+
             $captitle = new html_table_cell(get_capability_string($capability) . html_writer::span($capability));
             $captitle->header = true;
 
@@ -111,6 +117,10 @@ class tool_capability_renderer extends plugin_renderer_base {
             }
 
             $table->data[] = $row;
+            if (!empty($contexts[$contextid]->children)) {
+                $childcontextsids = array_merge($childcontextsids, $contexts[$contextid]->children);
+                $childcontextsids = array_unique($childcontextsids);
+            }
         }
 
         // Start the list item, and print the context name as a link to the place to make changes.
@@ -127,9 +137,9 @@ class tool_capability_renderer extends plugin_renderer_base {
         $html = $this->output->heading(html_writer::link($url, $context->get_context_name(), ['title' => $title]), 3);
         $html .= html_writer::table($table);
         // If there are any child contexts, print them recursively.
-        if (!empty($contexts[$contextid]->children)) {
-            foreach ($contexts[$contextid]->children as $childcontextid) {
-                $html .= $this->capability_comparison_table($capabilities, $childcontextid, $roles, true);
+        if (!empty($childcontextsids)) {
+            foreach ($childcontextsids as $childcontextid) {
+                $html .= $this->capability_comparison_table($capabilities, $childcontextid, $roles);
             }
         }
         return $html;

admin/tool/capability/tests/behat/show_contexts.feature

@@ -0,0 +1,97 @@
+@tool @tool_capability
+Feature: Show capabilities for multiple contexts
+  In order to check roles capabilities
+  As an admin
+  I need to be able to see capability overrides on several contexts
+
+  Background:
+    And the following "courses" exist:
+      | fullname | shortname | category |
+      | Course 1 | C1        | 0        |
+      | Course 2 | C2        | 0        |
+    And the following "permission overrides" exist:
+      | capability                    | permission | role       | contextlevel | reference |
+      | enrol/category:config         | Allow      | student    | Course       | C1        |
+      | enrol/cohort:unenrol          | Allow      | student    | Course       | C2        |
+    And I log in as "admin"
+    And I navigate to "Users > Permissions > Capability overview" in site administration
+
+  Scenario: Show capabilities table with one capability with overrides
+    When I set the following fields to these values:
+      | Capability: | enrol/category:config |
+      | Roles:      | Student               |
+    And I click on "Get the overview" "button"
+    Then I should see "System"
+    And I should see "Category: Miscellaneous"
+    And I should see "Course: Course 1"
+    And I should not see "Course: Course 2"
+
+  Scenario: Show capabilities table with one capability without overrides
+    When I set the following fields to these values:
+      | Capability: | enrol/cohort:config |
+      | Roles:      | Student               |
+    And I click on "Get the overview" "button"
+    Then I should see "System"
+    And I should not see "Category: Miscellaneous"
+    And I should not see "Course: Course 1"
+    And I should not see "Course: Course 2"
+
+  Scenario: Show capabilities table with two capabilities, 1st without overrides and 2nd with
+    When I set the following fields to these values:
+      | Capability: | enrol/category:synchronised, enrol/category:config |
+      | Roles:      | Student                                            |
+    And I click on "Get the overview" "button"
+    Then I should see "System"
+    And I should see "Category: Miscellaneous"
+    And I should see "Course: Course 1"
+    And I should not see "Course: Course 2"
+
+  Scenario: Show capabilities table with two capabilities, 1st with overrides and 2nd without
+    When I set the following fields to these values:
+      | Capability: | enrol/category:config, enrol/cohort:config |
+      | Roles:      | Student                                    |
+    And I click on "Get the overview" "button"
+    Then I should see "System"
+    And I should see "Category: Miscellaneous"
+    And I should see "Course: Course 1"
+    And I should not see "Course: Course 2"
+
+  Scenario: Show capabilities table with two capabilities, none with overrides
+    When I set the following fields to these values:
+      | Capability: | enrol/category:synchronised, enrol/cohort:config |
+      | Roles:      | Student                                          |
+    And I click on "Get the overview" "button"
+    Then I should see "System"
+    And I should not see "Category: Miscellaneous"
+    And I should not see "Course: Course 1"
+    And I should not see "Course: Course 2"
+
+  Scenario: Show capabilities table with capability with override and no role selected
+    When I set the following fields to these values:
+      | Capability: | enrol/category:config |
+      | Roles:      |                       |
+    And I click on "Get the overview" "button"
+    Then I should see "System"
+    And I should see "Category: Miscellaneous"
+    And I should see "Course: Course 1"
+    And I should not see "Course: Course 2"
+
+  Scenario: Show capabilities table with capability without override and no role selected
+    When I set the following fields to these values:
+      | Capability: | enrol/cohort:config |
+      | Roles:      |                     |
+    And I click on "Get the overview" "button"
+    Then I should see "System"
+    And I should not see "Category: Miscellaneous"
+    And I should not see "Course: Course 1"
+    And I should not see "Course: Course 2"
+
+  Scenario: Show capabilities table with capability with two overrides on different contexts
+    When I set the following fields to these values:
+      | Capability: | enrol/category:config, enrol/cohort:unenrol |
+      | Roles:      |                                           |
+    And I click on "Get the overview" "button"
+    Then I should see "System"
+    And I should see "Category: Miscellaneous"
+    And I should see "Course: Course 1"
+    And I should see "Course: Course 2"

admin/tool/cohortroles/classes/api.php

@@ -144,6 +144,10 @@ class api {
         $rolesadded = array();
         $rolesremoved = array();
 
+        // Remove any cohort role mappings for roles which have been deleted.
+        // The role assignments are not a consideration because these will have been removed when the role was.
+        $DB->delete_records_select('tool_cohortroles', "roleid NOT IN (SELECT id FROM {role})");
+
         // Get all cohort role assignments and group them by user and role.
         $all = cohort_role_assignment::get_records(array(), 'userid, roleid');
         // We build an better structure to loop on.
@@ -222,6 +226,48 @@ class api {
             }
         }
 
+        // Clean the legacy role assignments which are stale.
+        $paramsclean['usercontext'] = CONTEXT_USER;
+        $paramsclean['component'] = 'tool_cohortroles';
+        $sql = 'SELECT DISTINCT(ra.id), ra.roleid, ra.userid, ra.contextid, ctx.instanceid
+                  FROM {role_assignments} ra
+                  JOIN {context} ctx ON ctx.id = ra.contextid AND ctx.contextlevel = :usercontext
+                  JOIN {cohort_members} cm ON cm.userid = ctx.instanceid
+                  LEFT JOIN {tool_cohortroles} tc ON tc.cohortid = cm.cohortid
+                    AND tc.userid = ra.userid
+                    AND tc.roleid = ra.roleid
+                 WHERE ra.component = :component
+                   AND tc.id is null';
+        if ($candidatelegacyassignments = $DB->get_records_sql($sql, $paramsclean)) {
+            $sql = 'SELECT DISTINCT(ra.id)
+                  FROM {role_assignments} ra
+                  JOIN {context} ctx ON ctx.id = ra.contextid AND ctx.contextlevel = :usercontext
+                  JOIN {cohort_members} cm ON cm.userid = ctx.instanceid
+                  JOIN {tool_cohortroles} tc ON tc.cohortid = cm.cohortid AND tc.userid = ra.userid
+                 WHERE ra.component = :component';
+            if ($currentvalidroleassignments = $DB->get_records_sql($sql, $paramsclean)) {
+                foreach ($candidatelegacyassignments as $candidate) {
+                    if (!array_key_exists($candidate->id, $currentvalidroleassignments)) {
+                        role_unassign($candidate->roleid, $candidate->userid, $candidate->contextid, 'tool_cohortroles');
+                        $rolesremoved[] = array(
+                            'useridassignedto' => $candidate->userid,
+                            'useridassignedover' => $candidate->instanceid,
+                            'roleid' => $candidate->roleid
+                        );
+                    }
+                }
+            } else {
+                foreach ($candidatelegacyassignments as $candidate) {
+                    role_unassign($candidate->roleid, $candidate->userid, $candidate->contextid, 'tool_cohortroles');
+                    $rolesremoved[] = array(
+                        'useridassignedto' => $candidate->userid,
+                        'useridassignedover' => $candidate->instanceid,
+                        'roleid' => $candidate->roleid
+                    );
+                }
+            }
+        }
+
         return array('rolesadded' => $rolesadded, 'rolesremoved' => $rolesremoved);
     }
 

admin/tool/cohortroles/classes/cohort_role_assignment.php

@@ -103,5 +103,4 @@ class cohort_role_assignment extends persistent {
 
         return true;
     }
-
 }

admin/tool/cohortroles/db/upgrade.php

@@ -0,0 +1,45 @@
+<?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/>.
+
+/**
+ * Plugin upgrade code
+ *
+ * @package    tool_cohortroles
+ * @copyright  2020 Paul Holden <paulh@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Function to upgrade tool_cohortroles.
+ *
+ * @param int $oldversion the version we are upgrading from
+ * @return bool result
+ */
+function xmldb_tool_cohortroles_upgrade($oldversion) {
+    global $DB;
+
+    if ($oldversion < 2019052001) {
+        // Delete any tool_cohortroles mappings for roles which no longer exist.
+        $DB->delete_records_select('tool_cohortroles', 'roleid NOT IN (SELECT id FROM {role})');
+
+        // Cohortroles savepoint reached.
+        upgrade_plugin_savepoint(true, 2019052001, 'tool', 'cohortroles');
+    }
+
+    return true;
+}

admin/tool/cohortroles/tests/api_test.php

@@ -50,8 +50,6 @@ class tool_cohortroles_api_testcase extends advanced_testcase {
      * Setup function- we will create a course and add an assign instance to it.
      */
     protected function setUp() {
-        global $DB;
-
         $this->resetAfterTest(true);
 
         // Create some users.
@@ -133,14 +131,91 @@ class tool_cohortroles_api_testcase extends advanced_testcase {
 
     public function test_delete_cohort_role_assignment() {
         $this->setAdminUser();
-        $params = (object) array(
+        // Create a cohort role assigment.
+        $params = (object) [
             'userid' => $this->userassignto->id,
             'roleid' => $this->roleid,
             'cohortid' => $this->cohort->id
-        );
-        $result = api::create_cohort_role_assignment($params);
-        $worked = api::delete_cohort_role_assignment($result->get('id'));
-        $this->assertTrue($worked);
+        ];
+        $cohortroleassignment = api::create_cohort_role_assignment($params);
+        $sync = api::sync_all_cohort_roles();
+        $rolesadded = [
+            [
+                'useridassignedto' => $this->userassignto->id,
+                'useridassignedover' => $this->userassignover->id,
+                'roleid' => $this->roleid
+            ]
+        ];
+        $expected = [
+            'rolesadded' => $rolesadded,
+            'rolesremoved' => []
+        ];
+        $this->assertEquals($sync, $expected);
+
+        // Delete the cohort role assigment and confirm the roles are removed.
+        $result = api::delete_cohort_role_assignment($cohortroleassignment->get('id'));
+        $this->assertTrue($result);
+        $sync = api::sync_all_cohort_roles();
+        $expected = [
+            'rolesadded' => [],
+            'rolesremoved' => $rolesadded
+        ];
+        $this->assertEquals($expected, $sync);
+    }
+
+    /**
+     * Test case verifying that syncing won't remove role assignments if they are valid for another cohort role assignment.
+     */
+    public function test_delete_cohort_role_assignment_cohorts_having_same_members() {
+        $this->setAdminUser();
+
+        // Create 2 cohorts, with a 1 user (user1) present in both,
+        // and user2 and user3 members of 1 cohort each.
+        $cohort1 = $this->getDataGenerator()->create_cohort();
+        $cohort2 = $this->getDataGenerator()->create_cohort();
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+        $user3 = $this->getDataGenerator()->create_user();
+        cohort_add_member($cohort1->id, $user1->id);
+        cohort_add_member($cohort1->id, $user2->id);
+        cohort_add_member($cohort2->id, $user1->id);
+        cohort_add_member($cohort2->id, $user3->id);
+
+        // And a role and a user to assign that role to.
+        $user4 = $this->getDataGenerator()->create_user(); // A cohort manager, for example.
+        $roleid = create_role('Role 1', 'myrole', 'test');
+
+        // Assign the role for user4 in both cohorts.
+        $params = (object) [
+            'userid' => $user4->id,
+            'roleid' => $roleid,
+            'cohortid' => $cohort1->id
+        ];
+        $cohort1roleassignment = api::create_cohort_role_assignment($params);
+        $params->cohortid = $cohort2->id;
+        $cohort2roleassignment = api::create_cohort_role_assignment($params);
+
+        $sync = api::sync_all_cohort_roles();
+
+        // There is no guarantee about the order of roles assigned.
+        // so confirm we have 3 role assignments, and they are for the users 1, 2 and 3.
+        $this->assertCount(3, $sync['rolesadded']);
+        $addedusers = array_column($sync['rolesadded'], 'useridassignedover');
+        $this->assertContains($user1->id, $addedusers);
+        $this->assertContains($user2->id, $addedusers);
+        $this->assertContains($user3->id, $addedusers);
+
+        // Remove the role assignment for user4/cohort1.
+        // Verify only 1 role is unassigned as the others are still valid for the other cohort role assignment.
+        $result = api::delete_cohort_role_assignment($cohort1roleassignment->get('id'));
+        $this->assertTrue($result);
+
+        $sync = api::sync_all_cohort_roles();
+
+        $this->assertCount(0, $sync['rolesadded']);
+        $this->assertCount(1, $sync['rolesremoved']);
+        $removedusers = array_column($sync['rolesremoved'], 'useridassignedover');
+        $this->assertContains($user2->id, $removedusers);
     }
 
     public function test_list_cohort_role_assignments() {

admin/tool/cohortroles/version.php

@@ -25,7 +25,7 @@
 defined('MOODLE_INTERNAL') || die();
 
 
-$plugin->version   = 2019052000; // The current plugin version (Date: YYYYMMDDXX).
+$plugin->version   = 2019052001; // The current plugin version (Date: YYYYMMDDXX).
 $plugin->requires  = 2019051100; // Requires this Moodle version.
 $plugin->component = 'tool_cohortroles'; // Full name of the plugin (used for diagnostics).
 

admin/tool/dataprivacy/amd/build/expand_contract.min.js

@@ -1 +1 @@
-define(["jquery","core/url","core/str"],function(a,b,c){var d=a('<img alt="" src="'+b.imageUrl("t/expanded")+'"/>'),e=a('<img alt="" src="'+b.imageUrl("t/collapsed")+'"/>');return{expandCollapse:function(a,b){a.hasClass("hide")?(a.removeClass("hide"),a.addClass("visible"),a.attr("aria-expanded",!0),b.find(":header i.fa").removeClass("fa-plus-square"),b.find(":header i.fa").addClass("fa-minus-square"),b.find(":header img.icon").attr("src",d.attr("src"))):(a.removeClass("visible"),a.addClass("hide"),a.attr("aria-expanded",!1),b.find(":header i.fa").removeClass("fa-minus-square"),b.find(":header i.fa").addClass("fa-plus-square"),b.find(":header img.icon").attr("src",e.attr("src")))},expandCollapseAll:function(b){var f="visible"==b?"hide":"visible",g="visible"==b,h="visible"==b?"fa-plus-square":"fa-minus-square",i="visible"==b?"fa-minus-square":"fa-plus-square",j="visible"==b?d.attr("src"):e.attr("src");a("."+f).each(function(){a(this).removeClass(f),a(this).addClass(b),a(this).attr("aria-expanded",g)}),a(".tool_dataprivacy-expand-all").data("visibilityState",f),c.get_string(f,"tool_dataprivacy").then(function(b){a(".tool_dataprivacy-expand-all").html(b)})["catch"](Notification.exception),a(":header i.fa").each(function(){a(this).removeClass(h),a(this).addClass(i)}),a(":header img.icon").each(function(){a(this).attr("src",j)})}}});
+define(["jquery","core/url","core/str"],function(a,b,c){var d=a('<img alt="" src="'+b.imageUrl("t/expanded")+'"/>'),e=a('<img alt="" src="'+b.imageUrl("t/collapsed")+'"/>'),f={EXPAND:"fa-caret-right",COLLAPSE:"fa-caret-down"};return{expandCollapse:function(a,b){a.hasClass("hide")?(a.removeClass("hide"),a.addClass("visible"),a.attr("aria-expanded",!0),b.find(":header i.fa").removeClass(f.EXPAND),b.find(":header i.fa").addClass(f.COLLAPSE),b.find(":header img.icon").attr("src",d.attr("src"))):(a.removeClass("visible"),a.addClass("hide"),a.attr("aria-expanded",!1),b.find(":header i.fa").removeClass(f.COLLAPSE),b.find(":header i.fa").addClass(f.EXPAND),b.find(":header img.icon").attr("src",e.attr("src")))},expandCollapseAll:function(b){var g="visible"==b?"hide":"visible",h="visible"==b,i="visible"==b?f.EXPAND:f.COLLAPSE,j="visible"==b?f.COLLAPSE:f.EXPAND,k="visible"==b?d.attr("src"):e.attr("src");a("."+g).each(function(){a(this).removeClass(g),a(this).addClass(b),a(this).attr("aria-expanded",h)}),a(".tool_dataprivacy-expand-all").data("visibilityState",g),c.get_string(g,"tool_dataprivacy").then(function(b){a(".tool_dataprivacy-expand-all").html(b)})["catch"](Notification.exception),a(":header i.fa").each(function(){a(this).removeClass(i),a(this).addClass(j)}),a(":header img.icon").each(function(){a(this).attr("src",k)})}}});

admin/tool/dataprivacy/amd/build/myrequestactions.min.js

@@ -1 +1 @@
-define(["jquery","core/ajax","core/notification","core/str","core/modal_factory","core/modal_events","core/templates"],function(a,b,c,d,e,f,g){function h(a){var e={methodname:"tool_dataprivacy_contact_dpo",args:{message:a}},f="success";b.call([e])[0].then(function(a){return a.result?d.get_string("requestsubmitted","tool_dataprivacy"):(f="error",a.warnings.join("<br>"))}).done(function(a){c.addNotification({message:a,type:f})}).fail(c.exception)}var i={CANCEL_REQUEST:'[data-action="cancel"]',CONTACT_DPO:'[data-action="contactdpo"]'},j=function(){this.registerEvents()};return j.prototype.registerEvents=function(){a(i.CANCEL_REQUEST).click(function(g){g.preventDefault();var h=a(this).data("requestid"),i=[{key:"cancelrequest",component:"tool_dataprivacy"},{key:"cancelrequestconfirmation",component:"tool_dataprivacy"}];d.get_strings(i).then(function(a){var d=a[0],g=a[1];return e.create({title:d,body:g,type:e.types.SAVE_CANCEL}).then(function(a){return a.setSaveButtonText(d),a.getRoot().on(f.save,function(){var a={requestid:h},d={methodname:"tool_dataprivacy_cancel_data_request",args:a};b.call([d])[0].done(function(a){a.result?window.location.reload():c.addNotification({message:a.warnings[0].message,type:"error"})}).fail(c.exception)}),a.getRoot().on(f.hidden,function(){a.destroy()}),a})}).done(function(a){a.show()}).fail(c.exception)}),a(i.CONTACT_DPO).click(function(b){b.preventDefault();var i=a(this).data("replytoemail"),j=[{key:"contactdataprotectionofficer",component:"tool_dataprivacy"},{key:"send",component:"tool_dataprivacy"}],k="";d.get_strings(j).then(function(a){var b=a[0];k=a[1];var c={replytoemail:i};return e.create({title:b,body:g.render("tool_dataprivacy/contact_dpo",c),type:e.types.SAVE_CANCEL,large:!0})}).done(function(b){b.setSaveButtonText(k),b.getRoot().on(f.save,function(b){var c=a("#message").val().trim();0===c.length?(b.preventDefault(),a('[data-region="messageinput"]').addClass("has-danger notifyproblem"),a("#id_error_message").removeAttr("hidden")):h(c)}),b.getRoot().on(f.hidden,function(){b.destroy()}),b.show()}).fail(c.exception)})},{init:function(){return new j}}});
+define(["jquery","core/ajax","core/notification","core/str","core/modal_factory","core/modal_events","core/templates","core/pending"],function(a,b,c,d,e,f,g,h){function i(a){var e={methodname:"tool_dataprivacy_contact_dpo",args:{message:a}},f="success";b.call([e])[0].then(function(a){return a.result?d.get_string("requestsubmitted","tool_dataprivacy"):(f="error",a.warnings.join("<br>"))}).done(function(a){c.addNotification({message:a,type:f})}).fail(c.exception)}var j={CANCEL_REQUEST:'[data-action="cancel"]',CONTACT_DPO:'[data-action="contactdpo"]'},k=function(){this.registerEvents()};return k.prototype.registerEvents=function(){a(j.CANCEL_REQUEST).click(function(g){g.preventDefault();var h=a(this).data("requestid"),i=[{key:"cancelrequest",component:"tool_dataprivacy"},{key:"cancelrequestconfirmation",component:"tool_dataprivacy"}];d.get_strings(i).then(function(a){var d=a[0],g=a[1];return e.create({title:d,body:g,type:e.types.SAVE_CANCEL}).then(function(a){return a.setSaveButtonText(d),a.getRoot().on(f.save,function(){var a={requestid:h},d={methodname:"tool_dataprivacy_cancel_data_request",args:a};b.call([d])[0].done(function(a){a.result?window.location.reload():c.addNotification({message:a.warnings[0].message,type:"error"})}).fail(c.exception)}),a.getRoot().on(f.hidden,function(){a.destroy()}),a})}).done(function(a){a.show()}).fail(c.exception)}),a(j.CONTACT_DPO).click(function(b){var j=new h("dataprivacy/crud:initModal:contactdpo");b.preventDefault();var k=a(this).data("replytoemail"),l=[{key:"contactdataprotectionofficer",component:"tool_dataprivacy"},{key:"send",component:"tool_dataprivacy"}],m="";d.get_strings(l).then(function(a){var b=a[0];m=a[1];var c={replytoemail:k};return e.create({title:b,body:g.render("tool_dataprivacy/contact_dpo",c),type:e.types.SAVE_CANCEL,large:!0})}).then(function(b){b.setSaveButtonText(m),b.show(),b.getRoot().on(f.save,function(b){var c=a("#message").val().trim();0===c.length?(b.preventDefault(),a('[data-region="messageinput"]').addClass("has-danger notifyproblem"),a("#id_error_message").removeAttr("hidden")):i(c)}),b.getRoot().on(f.hidden,function(){b.destroy()})}).then(j.resolve)["catch"](c.exception)})},{init:function(){return new k}}});

admin/tool/dataprivacy/amd/src/expand_contract.js

@@ -28,6 +28,14 @@ define(['jquery', 'core/url', 'core/str'], function($, url, str) {
     var expandedImage = $('<img alt="" src="' + url.imageUrl('t/expanded') + '"/>');
     var collapsedImage = $('<img alt="" src="' + url.imageUrl('t/collapsed') + '"/>');
 
+    /*
+     * Class names to apply when expanding/collapsing nodes.
+     */
+    var CLASSES = {
+        EXPAND: 'fa-caret-right',
+        COLLAPSE: 'fa-caret-down'
+    };
+
     return /** @alias module:tool_dataprivacy/expand-collapse */ {
         /**
          * Expand or collapse a selected node.
@@ -40,15 +48,15 @@ define(['jquery', 'core/url', 'core/str'], function($, url, str) {
                 targetnode.removeClass('hide');
                 targetnode.addClass('visible');
                 targetnode.attr('aria-expanded', true);
-                thisnode.find(':header i.fa').removeClass('fa-plus-square');
-                thisnode.find(':header i.fa').addClass('fa-minus-square');
+                thisnode.find(':header i.fa').removeClass(CLASSES.EXPAND);
+                thisnode.find(':header i.fa').addClass(CLASSES.COLLAPSE);
                 thisnode.find(':header img.icon').attr('src', expandedImage.attr('src'));
             } else {
                 targetnode.removeClass('visible');
                 targetnode.addClass('hide');
                 targetnode.attr('aria-expanded', false);
-                thisnode.find(':header i.fa').removeClass('fa-minus-square');
-                thisnode.find(':header i.fa').addClass('fa-plus-square');
+                thisnode.find(':header i.fa').removeClass(CLASSES.COLLAPSE);
+                thisnode.find(':header i.fa').addClass(CLASSES.EXPAND);
                 thisnode.find(':header img.icon').attr('src', collapsedImage.attr('src'));
             }
         },
@@ -61,8 +69,8 @@ define(['jquery', 'core/url', 'core/str'], function($, url, str) {
         expandCollapseAll: function(nextstate) {
             var currentstate = (nextstate == 'visible') ? 'hide' : 'visible';
             var ariaexpandedstate = (nextstate == 'visible') ? true : false;
-            var iconclassnow = (nextstate == 'visible') ? 'fa-plus-square' : 'fa-minus-square';
-            var iconclassnext = (nextstate == 'visible') ? 'fa-minus-square' : 'fa-plus-square';
+            var iconclassnow = (nextstate == 'visible') ? CLASSES.EXPAND : CLASSES.COLLAPSE;
+            var iconclassnext = (nextstate == 'visible') ? CLASSES.COLLAPSE : CLASSES.EXPAND;
             var imagenow = (nextstate == 'visible') ? expandedImage.attr('src') : collapsedImage.attr('src');
             $('.' + currentstate).each(function() {
                 $(this).removeClass(currentstate);

admin/tool/dataprivacy/amd/src/myrequestactions.js

@@ -28,8 +28,9 @@ define([
     'core/str',
     'core/modal_factory',
     'core/modal_events',
-    'core/templates'],
-function($, Ajax, Notification, Str, ModalFactory, ModalEvents, Templates) {
+    'core/templates',
+    'core/pending'],
+function($, Ajax, Notification, Str, ModalFactory, ModalEvents, Templates, Pending) {
 
     /**
      * List of action selectors.
@@ -118,6 +119,7 @@ function($, Ajax, Notification, Str, ModalFactory, ModalEvents, Templates) {
         });
 
         $(ACTIONS.CONTACT_DPO).click(function(e) {
+            var pendingPromise = new Pending('dataprivacy/crud:initModal:contactdpo');
             e.preventDefault();
 
             var replyToEmail = $(this).data('replytoemail');
@@ -146,9 +148,12 @@ function($, Ajax, Notification, Str, ModalFactory, ModalEvents, Templates) {
                     type: ModalFactory.types.SAVE_CANCEL,
                     large: true
                 });
-            }).done(function(modal) {
+            }).then(function(modal) {
                 modal.setSaveButtonText(sendButtonText);
 
+                // Show the modal!
+                modal.show();
+
                 // Handle send event.
                 modal.getRoot().on(ModalEvents.save, function(e) {
                     var message = $('#message').val().trim();
@@ -169,9 +174,9 @@ function($, Ajax, Notification, Str, ModalFactory, ModalEvents, Templates) {
                     modal.destroy();
                 });
 
-                // Show the modal!
-                modal.show();
-            }).fail(Notification.exception);
+                return;
+            }).then(pendingPromise.resolve)
+            .catch(Notification.exception);
         });
     };
 

admin/tool/dataprivacy/classes/api.php

@@ -712,7 +712,7 @@ class api {
             'requestedby' => $requestedby->fullname,
             'requesttype' => $typetext,
             'requestdate' => userdate($requestdata->timecreated),
-            'requestorigin' => $SITE->fullname,
+            'requestorigin' => format_string($SITE->fullname, true, ['context' => context_system::instance()]),
             'requestoriginurl' => new moodle_url('/'),
             'requestcomments' => $requestdata->messagehtml,
             'datarequestsurl' => $datarequestsurl
@@ -774,7 +774,8 @@ class api {
     public static function can_create_data_deletion_request_for_self(int $userid = null): bool {
         global $USER;
         $userid = $userid ?: $USER->id;
-        return has_capability('tool/dataprivacy:requestdelete', \context_user::instance($userid), $userid);
+        return has_capability('tool/dataprivacy:requestdelete', \context_user::instance($userid), $userid)
+            && !is_primary_admin($userid);
     }
 
     /**
@@ -803,7 +804,7 @@ class api {
         global $USER;
         $requesterid = $requesterid ?: $USER->id;
         return has_capability('tool/dataprivacy:makedatadeletionrequestsforchildren', \context_user::instance($userid),
-            $requesterid);
+            $requesterid) && !is_primary_admin($userid);
     }
 
     /**

admin/tool/dataprivacy/classes/external.php

@@ -713,7 +713,6 @@ class external extends external_api {
 
         list($sql, $params) = users_search_sql($query, '', false, $extrafields, $excludedusers);
         $users = $DB->get_records_select('user', $sql, $params, $sort, $fields, 0, 30);
-
         $useroptions = [];
         foreach ($users as $user) {
             $useroption = (object)[
@@ -722,9 +721,10 @@ class external extends external_api {
             ];
             $useroption->extrafields = [];
             foreach ($extrafields as $extrafield) {
+                // Sanitize the extra fields to prevent potential XSS exploit.
                 $useroption->extrafields[] = (object)[
                     'name' => $extrafield,
-                    'value' => $user->$extrafield
+                    'value' => s($user->$extrafield)
                 ];
             }
             $useroptions[$user->id] = $useroption;

admin/tool/dataprivacy/classes/task/process_data_request_task.php

@@ -26,6 +26,7 @@ namespace tool_dataprivacy\task;
 
 use action_link;
 use coding_exception;
+use context_system;
 use core\message\message;
 use core\task\adhoc_task;
 use core_user;
@@ -127,13 +128,17 @@ class process_data_request_task extends adhoc_task {
             $thing = $fs->create_file_from_pathname($filerecord, $exportedcontent);
             $completestatus = api::DATAREQUEST_STATUS_DOWNLOAD_READY;
         } else if ($request->type == api::DATAREQUEST_TYPE_DELETE) {
-            // Delete the data.
-            $manager = new \core_privacy\manager();
-            $manager->set_observer(new \tool_dataprivacy\manager_observer());
+            // Delete the data for users other than the primary admin, which is rejected.
+            if (is_primary_admin($foruser->id)) {
+                $completestatus = api::DATAREQUEST_STATUS_REJECTED;
+            } else {
+                $manager = new \core_privacy\manager();
+                $manager->set_observer(new \tool_dataprivacy\manager_observer());
 
-            $manager->delete_data_for_user($approvedclcollection);
-            $completestatus = api::DATAREQUEST_STATUS_DELETED;
-            $deleteuser = !$foruser->deleted;
+                $manager->delete_data_for_user($approvedclcollection);
+                $completestatus = api::DATAREQUEST_STATUS_DELETED;
+                $deleteuser = !$foruser->deleted;
+            }
         }
 
         // When the preparation of the metadata finishes, update the request status to awaiting approval.
@@ -148,7 +153,7 @@ class process_data_request_task extends adhoc_task {
         $message->name = 'datarequestprocessingresults';
         $message->userfrom = $dpo;
         $message->replyto = $dpo->email;
-        $message->replytoname = fullname($dpo->email);
+        $message->replytoname = fullname($dpo);
 
         $typetext = null;
         // Prepare the context data for the email message body.
@@ -176,7 +181,8 @@ class process_data_request_task extends adhoc_task {
                 $message->contexturl = $datarequestsurl;
                 $message->contexturlname = get_string('datarequests', 'tool_dataprivacy');
                 // Message to the recipient.
-                $messagetextdata['message'] = get_string('resultdownloadready', 'tool_dataprivacy', $SITE->fullname);
+                $messagetextdata['message'] = get_string('resultdownloadready', 'tool_dataprivacy',
+                    format_string($SITE->fullname, true, ['context' => context_system::instance()]));
                 // Prepare download link.
                 $downloadurl = moodle_url::make_pluginfile_url($usercontext->id, 'tool_dataprivacy', 'export', $thing->get_itemid(),
                     $thing->get_filepath(), $thing->get_filename(), true);
@@ -188,7 +194,8 @@ class process_data_request_task extends adhoc_task {
                 // No point notifying a deleted user in Moodle.
                 $message->notification = 0;
                 // Message to the recipient.
-                $messagetextdata['message'] = get_string('resultdeleted', 'tool_dataprivacy', $SITE->fullname);
+                $messagetextdata['message'] = get_string('resultdeleted', 'tool_dataprivacy',
+                    format_string($SITE->fullname, true, ['context' => context_system::instance()]));
                 // Message will be sent to the deleted user via email only.
                 $emailonly = true;
                 break;

admin/tool/dataprivacy/createdatarequest.php

@@ -45,13 +45,22 @@ if ($manage) {
     $context = context_system::instance();
     // Make sure the user has the proper capability.
     require_capability('tool/dataprivacy:managedatarequests', $context);
+    navigation_node::override_active_url($returnurl);
 } else {
     // For the case where a user makes request for themselves (or for their children if they are the parent).
     $returnurl = new moodle_url($CFG->wwwroot . '/admin/tool/dataprivacy/mydatarequests.php');
     $context = context_user::instance($USER->id);
 }
+
 $PAGE->set_context($context);
 
+if (!$manage && $profilenode = $PAGE->settingsnav->find('myprofile', null)) {
+    $profilenode->make_active();
+}
+
+$title = get_string('createnewdatarequest', 'tool_dataprivacy');
+$PAGE->navbar->add($title);
+
 // If contactdataprotectionofficer is disabled, send the user back to the profile page, or the privacy policy page.
 // That is, unless you have sufficient capabilities to perform this on behalf of a user.
 if (!$manage && !\tool_dataprivacy\api::can_contact_dpo()) {
@@ -100,7 +109,6 @@ if ($data = $mform->get_data()) {
     redirect($returnurl, $redirectmessage);
 }
 
-$title = get_string('createnewdatarequest', 'tool_dataprivacy');
 $PAGE->set_heading($SITE->fullname);
 $PAGE->set_title($title);
 echo $OUTPUT->header();

admin/tool/dataprivacy/db/upgrade.php

@@ -319,5 +319,28 @@ function xmldb_tool_dataprivacy_upgrade($oldversion) {
     // Automatically generated Moodle v3.7.0 release upgrade line.
     // Put any upgrade step following this.
 
+    if ($oldversion < 2019052001) {
+
+        // Define field commentsformat to be added to tool_dataprivacy_request.
+        $table = new xmldb_table('tool_dataprivacy_request');
+        $field = new xmldb_field('commentsformat', XMLDB_TYPE_INTEGER, '2', null, XMLDB_NOTNULL, null, '0', 'comments');
+
+        // Conditionally launch add field commentsformat.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // Define field dpocommentformat to be added to tool_dataprivacy_request.
+        $field = new xmldb_field('dpocommentformat', XMLDB_TYPE_INTEGER, '2', null, XMLDB_NOTNULL, null, '0', 'dpocomment');
+
+        // Conditionally launch add field dpocommentformat.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // Dataprivacy savepoint reached.
+        upgrade_plugin_savepoint(true, 2019052001, 'tool', 'dataprivacy');
+    }
+
     return true;
 }

admin/tool/dataprivacy/mydatarequests.php

@@ -42,6 +42,13 @@ if (isguestuser()) {
 $usercontext = context_user::instance($USER->id);
 $PAGE->set_context($usercontext);
 
+if ($profilenode = $PAGE->settingsnav->find('myprofile', null)) {
+    $profilenode->make_active();
+}
+
+$title = get_string('datarequests', 'tool_dataprivacy');
+$PAGE->navbar->add($title);
+
 // Return URL.
 $params = ['id' => $USER->id];
 if ($courseid) {
@@ -49,7 +56,6 @@ if ($courseid) {
 }
 $returnurl = new moodle_url('/user/profile.php', $params);
 
-$title = get_string('datarequests', 'tool_dataprivacy');
 $PAGE->set_heading($title);
 $PAGE->set_title($title);
 echo $OUTPUT->header();

admin/tool/dataprivacy/resubmitrequest.php

@@ -27,12 +27,10 @@ require_once('../../../config.php');
 $requestid = required_param('requestid', PARAM_INT);
 $confirm = optional_param('confirm', null, PARAM_INT);
 
-$PAGE->set_url(new moodle_url('/admin/tool/dataprivacy/resubmitrequest.php', ['requestid' => $requestid]));
+$url = new moodle_url('/admin/tool/dataprivacy/resubmitrequest.php', ['requestid' => $requestid]);
+$title = get_string('resubmitrequestasnew', 'tool_dataprivacy');
 
-require_login();
-
-$PAGE->set_context(\context_system::instance());
-require_capability('tool/dataprivacy:managedatarequests', $PAGE->context);
+\tool_dataprivacy\page_helper::setup($url, $title, 'datarequests', 'tool/dataprivacy:managedatarequests');
 
 $manageurl = new moodle_url('/admin/tool/dataprivacy/datarequests.php');
 
@@ -53,10 +51,6 @@ if (null !== $confirm && confirm_sesskey()) {
     redirect($manageurl, get_string('resubmittedrequest', 'tool_dataprivacy', $stringparams));
 }
 
-$heading = get_string('resubmitrequest', 'tool_dataprivacy', $stringparams);
-$PAGE->set_title($heading);
-$PAGE->set_heading($heading);
-
 echo $OUTPUT->header();
 
 $confirmstring = get_string('confirmrequestresubmit', 'tool_dataprivacy', $stringparams);

admin/tool/dataprivacy/summary.php

@@ -34,6 +34,13 @@ $PAGE->set_context($context);
 $PAGE->set_title($title);
 $PAGE->set_heading($SITE->fullname);
 
+// If user is logged in, then use profile navigation in breadcrumbs.
+if ($profilenode = $PAGE->settingsnav->find('myprofile', null)) {
+    $profilenode->make_active();
+}
+
+$PAGE->navbar->add($title);
+
 $output = $PAGE->get_renderer('tool_dataprivacy');
 echo $output->header();
 $summarypage = new \tool_dataprivacy\output\summary_page();

admin/tool/dataprivacy/templates/form-user-selector-suggestion.mustache

@@ -47,6 +47,6 @@
 <span>
     <span>{{fullname}}</span>
     {{#extrafields}}
-        <span><small>{{value}}</small></span>
+        <span><small>{{{value}}}</small></span>
     {{/extrafields}}
 </span>

admin/tool/dataprivacy/tests/api_test.php

@@ -301,6 +301,34 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $datarequest = api::create_data_request($student->id, api::DATAREQUEST_TYPE_EXPORT);
 
         $requestid = $datarequest->get('id');
+
+        // Login as a user without DPO role.
+        $this->setUser($teacher);
+        $this->expectException(required_capability_exception::class);
+        api::approve_data_request($requestid);
+    }
+
+    /**
+     * Test that deletion requests for the primary admin are rejected
+     */
+    public function test_reject_data_deletion_request_primary_admin() {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $datarequest = api::create_data_request(get_admin()->id, api::DATAREQUEST_TYPE_DELETE);
+
+        // Approve the request and execute the ad-hoc process task.
+        ob_start();
+        api::approve_data_request($datarequest->get('id'));
+        $this->runAdhocTasks('\tool_dataprivacy\task\process_data_request_task');
+        ob_end_clean();
+
+        $request = api::get_request($datarequest->get('id'));
+        $this->assertEquals(api::DATAREQUEST_STATUS_REJECTED, $request->get('status'));
+
+        // Confirm they weren't deleted.
+        $user = core_user::get_user($request->get('userid'));
+        core_user::require_active_user($user);
     }
 
     /**
@@ -2126,6 +2154,33 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $this->assertFalse(api::can_create_data_deletion_request_for_self());
     }
 
+    /**
+     * Test primary admin cannot create data deletion request for themselves
+     */
+    public function test_can_create_data_deletion_request_for_self_primary_admin() {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+        $this->assertFalse(api::can_create_data_deletion_request_for_self());
+    }
+
+    /**
+     * Test secondary admin can create data deletion request for themselves
+     */
+    public function test_can_create_data_deletion_request_for_self_secondary_admin() {
+        $this->resetAfterTest();
+
+        $admin1 = $this->getDataGenerator()->create_user();
+        $admin2 = $this->getDataGenerator()->create_user();
+
+        // The primary admin is the one listed first in the 'siteadmins' config.
+        set_config('siteadmins', implode(',', [$admin1->id, $admin2->id]));
+
+        // Set the current user as the second admin (non-primary).
+        $this->setUser($admin2);
+
+        $this->assertTrue(api::can_create_data_deletion_request_for_self());
+    }
+
     /**
      * Test user can create data deletion request for themselves if they have
      * "tool/dataprivacy:requestdelete" capability.
@@ -2171,7 +2226,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
     }
 
     /**
-     * Check parents can create data deletion request for their children but not others.
+     * Check parents can create data deletion request for their children (unless the child is the primary admin),
+     * but not other users.
      *
      * @throws coding_exception
      * @throws dml_exception
@@ -2194,5 +2250,9 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $this->setUser($parent);
         $this->assertTrue(api::can_create_data_deletion_request_for_children($child->id));
         $this->assertFalse(api::can_create_data_deletion_request_for_children($otheruser->id));
+
+        // Now make child the primary admin, confirm parent can't make deletion request.
+        set_config('siteadmins', $child->id);
+        $this->assertFalse(api::can_create_data_deletion_request_for_children($child->id));
     }
 }

admin/tool/dataprivacy/tests/behat/datadelete.feature

@@ -189,6 +189,12 @@ Feature: Data delete from the privacy API
     And I follow "Profile" in the user menu
     Then I should not see "Delete my account"
 
+  @javascript
+  Scenario: As a primary admin, the link to create a data deletion request should not be shown.
+    Given I log in as "admin"
+    When I follow "Profile" in the user menu
+    Then I should not see "Delete my account"
+
   @javascript
   Scenario: As a Privacy Officer, I cannot Approve to Deny deletion data request without permission.
     Given the following "permission overrides" exist:

admin/tool/dataprivacy/tests/behat/manage_categories.feature

@@ -12,7 +12,7 @@ Feature: Manage data categories
     And I press "Add category"
     And I set the field "Name" to "Category 1"
     And I set the field "Description" to "Category 1 description"
-    When I click on "Save" "button" in the "Delete category" "dialogue"
+    When I click on "Save" "button" in the "Add category" "dialogue"
     Then I should see "Category 1" in the "List of data categories" "table"
     And I should see "Category 1 description" in the "Category 1" "table_row"
 
@@ -30,5 +30,5 @@ Feature: Manage data categories
     And I choose "Delete" in the open action menu
     And I should see "Delete category"
     And I should see "Are you sure you want to delete the category 'Category 1'?"
-    When I click on "Delete" "button" in the "Confirm" "dialogue"
+    When I click on "Delete" "button" in the "Delete category" "dialogue"
     Then I should not see "Category 1" in the "List of data categories" "table"

admin/tool/dataprivacy/tests/behat/manage_purposes.feature

@@ -52,5 +52,5 @@ Feature: Manage data storage purposes
     And I choose "Delete" in the open action menu
     And I should see "Delete purpose"
     And I should see "Are you sure you want to delete the purpose 'Purpose 1'?"
-    When I click on "Delete" "button" in the "Confirm" "dialogue"
+    When I click on "Delete" "button" in the "Delete purpose" "dialogue"
     Then I should not see "Purpose 1" in the "List of data purposes" "table"

admin/tool/dataprivacy/tests/privacy_provider_test.php

@@ -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);
+    }
+}

admin/tool/dataprivacy/version.php

@@ -24,6 +24,6 @@
 
 defined('MOODLE_INTERNAL') || die;
 
-$plugin->version   = 2019052000;
+$plugin->version   = 2019052001;
 $plugin->requires  = 2019051100;        // Moodle 3.5dev (Build 2018031600) and upwards.
 $plugin->component = 'tool_dataprivacy';

admin/tool/lp/amd/build/dialogue.min.js

@@ -1 +1 @@
-define(["core/yui"],function(a){var b=function(b,c,d,e,f){this.yuiDialogue=null;var g=this;"undefined"==typeof f&&(f=!1),a.use("moodle-core-notification","timers",function(){var h="480px";f&&(h="800px"),g.yuiDialogue=new M.core.dialogue({headerContent:b,bodyContent:c,draggable:!0,visible:!1,center:!0,modal:!0,width:h}),g.yuiDialogue.after("visibleChange",function(b){b.newVal?"undefined"!=typeof d&&a.soon(function(){d(g),g.yuiDialogue.centerDialogue()}):"undefined"!=typeof e&&a.soon(function(){e(g)})}),g.yuiDialogue.show()})};return b.prototype.close=function(){this.yuiDialogue.hide(),this.yuiDialogue.destroy()},b.prototype.getContent=function(){return this.yuiDialogue.bodyNode.getDOMNode()},b});
+define(["core/yui"],function(a){var b=function(b,c,d,e,f){M.util.js_pending("tool_lp/dialogue:dialogue"),this.yuiDialogue=null;var g=this;"undefined"==typeof f&&(f=!1),a.use("moodle-core-notification","timers",function(){var h="480px";f&&(h="800px"),g.yuiDialogue=new M.core.dialogue({headerContent:b,bodyContent:c,draggable:!0,visible:!1,center:!0,modal:!0,width:h}),g.yuiDialogue.before("visibleChange",function(){M.util.js_pending("tool_lp/dialogue:before:visibleChange")}),g.yuiDialogue.after("visibleChange",function(b){b.newVal?"undefined"!=typeof d?a.soon(function(){d(g),g.yuiDialogue.centerDialogue(),M.util.js_complete("tool_lp/dialogue:before:visibleChange")}):M.util.js_complete("tool_lp/dialogue:before:visibleChange"):"undefined"!=typeof e?a.soon(function(){e(g),M.util.js_complete("tool_lp/dialogue:before:visibleChange")}):M.util.js_complete("tool_lp/dialogue:before:visibleChange")}),g.yuiDialogue.show(),M.util.js_complete("tool_lp/dialogue:dialogue")})};return b.prototype.close=function(){this.yuiDialogue.hide(),this.yuiDialogue.destroy()},b.prototype.getContent=function(){return this.yuiDialogue.bodyNode.getDOMNode()},b});

admin/tool/lp/amd/build/grade_dialogue.min.js

@@ -1 +1 @@
-define(["jquery","core/notification","core/templates","tool_lp/dialogue","tool_lp/event_base","core/str"],function(a,b,c,d,e,f){var g=function(a){e.prototype.constructor.apply(this,[]),this._ratingOptions=a};return g.prototype=Object.create(e.prototype),g.prototype._popup=null,g.prototype._ratingOptions=null,g.prototype._afterRender=function(){var b=this._find('[data-action="rate"]'),c=this._find('[name="rating"]'),d=this._find('[name="comment"]');this._find('[data-action="cancel"]').click(function(a){a.preventDefault(),this._trigger("cancelled"),this.close()}.bind(this)),c.change(function(){var c=a(this);c.val()?b.prop("disabled",!1):b.prop("disabled",!0)}).change(),b.click(function(a){a.preventDefault();var b=c.val();b&&(this._trigger("rated",{rating:b,note:d.val()}),this.close())}.bind(this))},g.prototype.close=function(){this._popup.close(),this._popup=null},g.prototype.display=function(){return a.when(f.get_string("rate","tool_lp"),this._render()).then(function(a,b){return this._popup=new d(a,b[0],this._afterRender.bind(this)),this._popup}.bind(this))["catch"](b.exception)},g.prototype._find=function(b){return a(this._popup.getContent()).find(b)},g.prototype._render=function(){var a={cangrade:this._canGrade,ratings:this._ratingOptions};return c.render("tool_lp/competency_grader",a)},g});
+define(["jquery","core/notification","core/templates","tool_lp/dialogue","tool_lp/event_base","core/str"],function(a,b,c,d,e,f){var g=function(a){e.prototype.constructor.apply(this,[]),this._ratingOptions=a};return g.prototype=Object.create(e.prototype),g.prototype._popup=null,g.prototype._ratingOptions=null,g.prototype._afterRender=function(){var b=this._find('[data-action="rate"]'),c=this._find('[name="rating"]'),d=this._find('[name="comment"]');this._find('[data-action="cancel"]').click(function(a){a.preventDefault(),this._trigger("cancelled"),this.close()}.bind(this)),c.change(function(){var c=a(this);c.val()?b.prop("disabled",!1):b.prop("disabled",!0)}).change(),b.click(function(a){a.preventDefault();var b=c.val();b&&(this._trigger("rated",{rating:b,note:d.val()}),this.close())}.bind(this))},g.prototype.close=function(){this._popup.close(),this._popup=null},g.prototype.display=function(){return M.util.js_pending("tool_lp/grade_dialogue:display"),a.when(f.get_string("rate","tool_lp"),this._render()).then(function(a,b){return this._popup=new d(a,b[0],function(){this._afterRender(),M.util.js_complete("tool_lp/grade_dialogue:display")}.bind(this)),this._popup}.bind(this))["catch"](b.exception)},g.prototype._find=function(b){return a(this._popup.getContent()).find(b)},g.prototype._render=function(){var a={cangrade:this._canGrade,ratings:this._ratingOptions};return c.render("tool_lp/competency_grader",a)},g});

admin/tool/lp/amd/build/grade_user_competency_inline.min.js

@@ -1 +1 @@
-define(["jquery","core/notification","core/ajax","core/log","tool_lp/grade_dialogue","tool_lp/event_base","tool_lp/scalevalues"],function(a,b,c,d,e,f,g){var h=function(b,c,d,e,g,h,i){f.prototype.constructor.apply(this,[]);var j=a(b);if(!j.length)throw new Error("Could not find the trigger");this._scaleId=c,this._competencyId=d,this._userId=e,this._planId=g,this._courseId=h,this._chooseStr=i,this._setUp(),j.click(function(a){a.preventDefault(),this._dialogue.display()}.bind(this)),this._planId?(this._methodName="core_competency_grade_competency_in_plan",this._args={competencyid:this._competencyId,planid:this._planId}):this._courseId?(this._methodName="core_competency_grade_competency_in_course",this._args={competencyid:this._competencyId,courseid:this._courseId,userid:this._userId}):(this._methodName="core_competency_grade_competency",this._args={userid:this._userId,competencyid:this._competencyId})};return h.prototype=Object.create(f.prototype),h.prototype._setUp=function(){var a=[],d=this,f=g.get_values(d._scaleId);f.then(function(b){a.push({value:"",name:d._chooseStr});for(var c=0;c<b.length;c++){var e=b[c];a.push({value:e.id,name:e.name})}return a}).then(function(a){return new e(a)}).then(function(a){return a.on("rated",function(a,e){var f=d._args;f.grade=e.rating,f.note=e.note,c.call([{methodname:d._methodName,args:f,done:function(a){d._trigger("competencyupdated",{args:f,evidence:a})},fail:b.exception}])}),a}).then(function(a){d._dialogue=a}).fail(b.exception)},h.prototype._scaleId=null,h.prototype._competencyId=null,h.prototype._userId=null,h.prototype._planId=null,h.prototype._courseId=null,h.prototype._chooseStr=null,h.prototype._dialogue=null,h});
+define(["jquery","core/notification","core/ajax","core/log","tool_lp/grade_dialogue","tool_lp/event_base","tool_lp/scalevalues"],function(a,b,c,d,e,f,g){var h=function(b,c,d,e,g,h,i){f.prototype.constructor.apply(this,[]);var j=a(b);if(!j.length)throw new Error("Could not find the trigger");this._scaleId=c,this._competencyId=d,this._userId=e,this._planId=g,this._courseId=h,this._chooseStr=i,this._setUp(),j.click(function(a){a.preventDefault(),this._dialogue.display()}.bind(this)),this._planId?(this._methodName="core_competency_grade_competency_in_plan",this._args={competencyid:this._competencyId,planid:this._planId}):this._courseId?(this._methodName="core_competency_grade_competency_in_course",this._args={competencyid:this._competencyId,courseid:this._courseId,userid:this._userId}):(this._methodName="core_competency_grade_competency",this._args={userid:this._userId,competencyid:this._competencyId})};return h.prototype=Object.create(f.prototype),h.prototype._setUp=function(){var a=[],d=this;M.util.js_pending("tool_lp/grade_user_competency_inline:_setUp");var f=g.get_values(d._scaleId);f.then(function(b){a.push({value:"",name:d._chooseStr});for(var c=0;c<b.length;c++){var e=b[c];a.push({value:e.id,name:e.name})}return a}).then(function(a){return new e(a)}).then(function(a){return a.on("rated",function(a,e){var f=d._args;f.grade=e.rating,f.note=e.note,c.call([{methodname:d._methodName,args:f,done:function(a){d._trigger("competencyupdated",{args:f,evidence:a})},fail:b.exception}])}),a}).then(function(a){d._dialogue=a,M.util.js_complete("tool_lp/grade_user_competency_inline:_setUp")}).fail(b.exception)},h.prototype._scaleId=null,h.prototype._competencyId=null,h.prototype._userId=null,h.prototype._planId=null,h.prototype._courseId=null,h.prototype._chooseStr=null,h.prototype._dialogue=null,h});

admin/tool/lp/amd/src/dialogue.js

@@ -35,6 +35,8 @@ define(['core/yui'], function(Y) {
      * @param {Boolean} wide Specify we want an extra wide dialogue (the size is standard, but wider than the default).
      */
     var dialogue = function(title, content, afterShow, afterHide, wide) {
+        M.util.js_pending('tool_lp/dialogue:dialogue');
+
         this.yuiDialogue = null;
         var parent = this;
 
@@ -59,6 +61,10 @@ define(['core/yui'], function(Y) {
                 width: width
             });
 
+            parent.yuiDialogue.before('visibleChange', function() {
+                M.util.js_pending('tool_lp/dialogue:before:visibleChange');
+            });
+
             parent.yuiDialogue.after('visibleChange', function(e) {
                 if (e.newVal) {
                     // Delay the callback call to the next tick, otherwise it can happen that it is
@@ -67,18 +73,25 @@ define(['core/yui'], function(Y) {
                         Y.soon(function() {
                             afterShow(parent);
                             parent.yuiDialogue.centerDialogue();
+                            M.util.js_complete('tool_lp/dialogue:before:visibleChange');
                         });
+                    } else {
+                        M.util.js_complete('tool_lp/dialogue:before:visibleChange');
                     }
                 } else {
                     if ((typeof afterHide !== 'undefined')) {
                         Y.soon(function() {
                             afterHide(parent);
+                            M.util.js_complete('tool_lp/dialogue:before:visibleChange');
                         });
+                    } else {
+                        M.util.js_complete('tool_lp/dialogue:before:visibleChange');
                     }
                 }
             });
 
             parent.yuiDialogue.show();
+            M.util.js_complete('tool_lp/dialogue:dialogue');
         });
     };
 

admin/tool/lp/amd/src/grade_dialogue.js

@@ -102,6 +102,7 @@ define(['jquery',
      * @return {Promise}
      */
     Grade.prototype.display = function() {
+        M.util.js_pending('tool_lp/grade_dialogue:display');
         return $.when(
             Str.get_string('rate', 'tool_lp'),
             this._render()
@@ -110,7 +111,10 @@ define(['jquery',
             this._popup = new Dialogue(
                 title,
                 templateResult[0],
-                this._afterRender.bind(this)
+                function() {
+                    this._afterRender();
+                    M.util.js_complete('tool_lp/grade_dialogue:display');
+                }.bind(this)
             );
 
             return this._popup;

admin/tool/lp/amd/src/grade_user_competency_inline.js

@@ -94,6 +94,7 @@ define(['jquery',
         var options = [],
             self = this;
 
+        M.util.js_pending('tool_lp/grade_user_competency_inline:_setUp');
         var promise = ScaleValues.get_values(self._scaleId);
         promise.then(function(scalevalues) {
             options.push({
@@ -134,6 +135,7 @@ define(['jquery',
         .then(function(dialogue) {
             self._dialogue = dialogue;
 
+            M.util.js_complete('tool_lp/grade_user_competency_inline:_setUp');
             return;
         })
         .fail(notification.exception);

admin/tool/lp/classes/output/manage_competency_frameworks_page.php

@@ -75,7 +75,7 @@ class manage_competency_frameworks_page implements renderable, templatable {
             );
             $this->navigation[] = $addpage;
             $competenciesrepository = new single_button(
-                new moodle_url('https://moodle.net/competencies'),
+                new moodle_url('https://archive.moodle.net/competencies'),
                 get_string('competencyframeworksrepository', 'tool_lp'),
                 'get'
             );

admin/tool/lp/templates/competency_plan_navigation.mustache

@@ -32,7 +32,7 @@
 
     // No example context because the JS is connected to webservices
 }}
-<div class="float-sm-right card card-block">
+<div class="float-sm-right border p-2 mb-2">
 {{#hascompetencies}}
 <span>
 <label for="competency-nav-{{uniqid}}" class="accesshide">{{#str}}jumptocompetency, tool_lp{{/str}}</label>

admin/tool/lp/templates/course_competencies_page.mustache

@@ -82,7 +82,7 @@
         {{/comppath}}
         {{#usercompetencycourse}}
             {{#grade}}
-            <span class="badge {{^proficiency}}badge-danger{{/proficiency}}">{{gradename}}</span>
+            <span class="badge {{#proficiency}}badge-success{{/proficiency}}{{^proficiency}}badge-danger{{/proficiency}}">{{gradename}}</span>
             {{/grade}}
         {{/usercompetencycourse}}
         {{#canmanagecoursecompetencies}}

admin/tool/lp/templates/course_competency_statistics.mustache

@@ -49,7 +49,7 @@
     Template statistics template.
 }}
 {{#competencycount}}
-<div data-region="coursecompetencystatistics" class="card">
+<div data-region="coursecompetencystatistics" class="border p-2 mb-2">
     {{#canbegradedincourse}}
         {{< tool_lp/progress_bar}}
             {{$progresstext}}

admin/tool/lp/templates/evidence_summary.mustache

@@ -44,7 +44,7 @@
       "id": 1
     }
 }}
-<div class="card well-small evidence" data-region="evidence" data-id="{{id}}">
+<div class="border p-2 mb-2 evidence" data-region="evidence" data-id="{{id}}">
     {{#candelete}}
         <div class="float-sm-right">
             <a href="#" data-action="delete-evidence">{{#pix}}t/delete{{/pix}}</a>

admin/tool/lp/templates/manage_competencies_page.mustache

@@ -50,10 +50,12 @@
         <div class="col-lg-6">
             <p>
                 <form data-region="filtercompetencies" data-frameworkid="{{framework.id}}" class="form-inline">
-                    <div class="form-group">
+                    <div class="input-group mb-3">
                         <label class="accesshide" for="filter{{uniqid}}">{{#str}}search, tool_lp{{/str}}</label>
                         <input class="form-control" type="text" id="filter{{uniqid}}" placeholder="{{#str}}search, tool_lp{{/str}}" value="{{search}}">
-                        <button class="btn btn-secondary">{{#pix}}a/search, , {{#str}}search{{/str}}{{/pix}}</button>
+                        <div class="input-group-append">
+                            <button class="btn btn-secondary">{{#pix}}a/search, , {{#str}}search{{/str}}{{/pix}}</button>
+                        </div>
                     </div>
                 </form>
             </p>
@@ -62,8 +64,8 @@
             </ul>
         </div>
 
-        <div class="card col-lg-6">
-            <div class="card-block card-body">
+        <div class="col-lg-6">
+            <div class="border p-2 mb-2">
                 <div class="card-title">
                     <h4 data-region="selected-competency">{{#str}}selectedcompetency, tool_lp{{/str}}</h4>
                         <span data-region="competencyactionsmenu" class="float-sm-right">

admin/tool/lp/templates/module_navigation.mustache

@@ -28,7 +28,7 @@
 
     // No example context because the JS is connected to webservices
 }}
-<div class="float-right card p-2">
+<div class="float-right border mb-2 p-2">
 <form class="user-competency-course-navigation">
 <input type="hidden" name="courseid" value="{{courseid}}"/>
 <input type="hidden" name="mod" value="{{moduleid}}"/>

admin/tool/lp/templates/template_competencies_page.mustache

@@ -26,10 +26,8 @@
     </h2>
     <div>{{{template.description}}}</div>
     {{#canmanagetemplatecompetencies}}
-    <div data-region="actions" class="clearfix">
-        <div class="float-left">
-            <button disabled class="btn btn-secondary">{{#str}}addtemplatecompetencies, tool_lp{{/str}}</button>
-        </div>
+    <div data-region="actions" class="my-4">
+        <button disabled class="btn btn-secondary">{{#str}}addtemplatecompetencies, tool_lp{{/str}}</button>
     </div>
     {{/canmanagetemplatecompetencies}}
     <h3 class="mt-1">{{#str}}templatecompetencies, tool_lp{{/str}}</h3>
@@ -41,7 +39,7 @@
             <div class="drag-parentnode">
                 {{#competencies}}
                 <div class="drag-samenode" data-id="{{competency.id}}">
-                    <div class="card">
+                    <div class="border p-2 mb-2">
                         {{#canmanagetemplatecompetencies}}
                         <span class="drag-handlecontainer float-left"></span>
                         <div class="float-sm-right">

admin/tool/lp/templates/template_statistics.mustache

@@ -45,8 +45,7 @@
     Template statistics template.
 }}
 {{#competencycount}}
-<div data-region="templatestatistics" class="card">
-    <div class="card-block">
+<div data-region="templatestatistics" class="border p-2 mb-2">
     {{< tool_lp/progress_bar}}
         {{$progresstext}}
             {{#str}}xcompetencieslinkedoutofy, tool_lp, { "x": "{{linkedcompetencycount}}", "y": "{{competencycount}}" } {{/str}}
@@ -90,6 +89,5 @@
         </div>
     </div>
     {{/leastproficientcount}}
-    </div>
 </div>
 {{/competencycount}}

admin/tool/lp/templates/user_competency_course_navigation.mustache

@@ -35,7 +35,7 @@
 
     // No example context because the JS is connected to webservices
 }}
-<div class="float-sm-right card card-block p-x-1 p-b-1">
+<div class="float-sm-right border p-2 mb-2">
 <p>{{{groupselector}}}</p>
 <form class="user-competency-course-navigation">
 <input type="hidden" name="userid" value="{{userid}}"/>

admin/tool/lp/templates/user_competency_summary.mustache

@@ -57,7 +57,7 @@
         </dd>
         <dt>{{#str}}proficient, tool_lp{{/str}}</dt>
         <dd>
-            <span class="badge{{^proficiency}} badge-danger{{/proficiency}} float-left">
+            <span class="badge {{#proficiency}}badge-success{{/proficiency}}{{^proficiency}}badge-danger{{/proficiency}} float-left">
                 {{proficiencyname}}
             </span>
         </dd>

admin/tool/lp/templates/user_competency_summary_in_course.mustache

@@ -81,7 +81,7 @@
         {{#usercompetencycourse}}
         <dt>{{#str}}proficient, tool_lp{{/str}}</dt>
         <dd>
-            <span class="badge{{^proficiency}} badge-danger{{/proficiency}} float-left">
+            <span class="badge {{#proficiency}}badge-success{{/proficiency}}{{^proficiency}}badge-danger{{/proficiency}} float-left">
                 {{proficiencyname}}
             </span>
         </dd>

admin/tool/lp/templates/user_competency_summary_in_plan.mustache

@@ -68,7 +68,7 @@
         </dd>
         <dt>{{#str}}proficient, tool_lp{{/str}}</dt>
         <dd>
-            <span class="badge{{^proficiency}} badge-danger{{/proficiency}} float-left">
+            <span class="badge {{#proficiency}}badge-success{{/proficiency}}{{^proficiency}}badge-danger{{/proficiency}} float-left">
                 {{proficiencyname}}
             </span>
         </dd>
@@ -101,7 +101,7 @@
         <dd>{{gradename}} - {{#str}}plancompleted, tool_lp{{/str}}</dd>
         <dt>{{#str}}proficient, tool_lp{{/str}}</dt>
         <dd>
-            <span class="badge{{^proficiency}} badge-danger{{/proficiency}} float-left">
+            <span class="badge {{#proficiency}}badge-success{{/proficiency}}{{^proficiency}}badge-danger{{/proficiency}} float-left">
                 {{proficiencyname}}
             </span>
         </dd>

admin/tool/messageinbound/lang/en/tool_messageinbound.php

@@ -24,7 +24,7 @@
 
 $string['classname'] = 'Class name';
 $string['component'] = 'Component';
-$string['configmessageinboundhost'] = 'The address of the server that Moodle should check mail against. To specify a non-default port, use [server]:[port], for example mail.example.com:587. If a port isn\'t specified, the default port for the type of mail server will be used.';
+$string['configmessageinboundhost'] = 'The address of the server that Moodle should check mail against. To specify a non-default port, use [server]:[port], for example mail.example.com:993. If a port isn\'t specified, the default port for the type of mail server will be used.';
 $string['defaultexpiration'] = 'Default address expiry period';
 $string['defaultexpiration_help'] = 'When an email address is generated by the handler, it can be set to automatically expire after a period of time, so that it can no longer be used. It is advisable to set an expiry period.';
 $string['description'] = 'Description';

admin/tool/mobile/classes/api.php

@@ -49,6 +49,8 @@ class api {
     const LOGIN_VIA_EMBEDDED_BROWSER = 3;
     /** @var int seconds an auto-login key will expire. */
     const LOGIN_KEY_TTL = 60;
+    /** @var string URL of the Moodle Apps Portal */
+    const MOODLE_APPS_PORTAL_URL = 'https://apps.moodle.com';
 
     /**
      * Returns a list of Moodle plugins supporting the mobile app.
@@ -206,6 +208,12 @@ class api {
         $identityprovidersdata = \auth_plugin_base::prepare_identity_providers_for_output($identityproviders, $OUTPUT);
         if (!empty($identityprovidersdata)) {
             $settings['identityproviders'] = $identityprovidersdata;
+            // Clean URLs to avoid breaking Web Services.
+            // We can't do it in prepare_identity_providers_for_output() because it may break the web output.
+            foreach ($settings['identityproviders'] as &$ip) {
+                $ip['url'] = (!empty($ip['url'])) ? clean_param($ip['url'], PARAM_URL) : '';
+                $ip['iconurl'] = (!empty($ip['iconurl'])) ? clean_param($ip['iconurl'], PARAM_URL) : '';
+            }
         }
 
         // If age is verified, return also the admin contact details.
@@ -340,6 +348,7 @@ class api {
      */
     public static function get_features_list() {
         global $CFG;
+        require_once($CFG->libdir . '/authlib.php');
 
         $general = new lang_string('general');
         $mainmenu = new lang_string('mainmenu', 'tool_mobile');
@@ -349,6 +358,7 @@ class api {
         $user = new lang_string('user');
         $files = new lang_string('files');
         $remoteaddons = new lang_string('remoteaddons', 'tool_mobile');
+        $identityproviders = new lang_string('oauth2identityproviders', 'tool_mobile');
 
         $availablemods = core_plugin_manager::instance()->get_plugins_of_type('mod');
         $coursemodules = array();
@@ -415,6 +425,7 @@ class api {
                 'NoDelegate_CoreTag' => new lang_string('tags'),
                 '$mmLoginEmailSignup' => new lang_string('startsignup'),
                 'NoDelegate_ResponsiveMainMenuItems' => new lang_string('responsivemainmenuitems', 'tool_mobile'),
+                'NoDelegate_DarkMode' => new lang_string('darkmode', 'tool_mobile'),
             ),
             "$mainmenu" => array(
                 '$mmSideMenuDelegate_mmaFrontpage' => new lang_string('sitehome'),
@@ -467,6 +478,31 @@ class api {
             $features["$remoteaddons"] = $remoteaddonslist;
         }
 
+        // Display OAuth 2 identity providers.
+        if (is_enabled_auth('oauth2')) {
+            $identityproviderslist = array();
+            $idps = \auth_plugin_base::get_identity_providers(['oauth2']);
+
+            foreach ($idps as $idp) {
+                // Only add identity providers that have an ID.
+                $id = isset($idp['url']) ? $idp['url']->get_param('id') : null;
+                if ($id != null) {
+                    $identityproviderslist['NoDelegate_IdentityProvider_' . $id] = $idp['name'];
+                }
+            }
+
+            if (!empty($identityproviderslist)) {
+                $features["$identityproviders"] = array();
+
+                if (count($identityproviderslist) > 1) {
+                    // Include an option to disable them all.
+                    $features["$identityproviders"]['NoDelegate_IdentityProviders'] = new lang_string('all');
+                }
+
+                $features["$identityproviders"] = array_merge($features["$identityproviders"], $identityproviderslist);
+            }
+        }
+
         return $features;
     }
 

admin/tool/mobile/lang/en/tool_mobile.php

@@ -45,12 +45,16 @@ $string['custommenuitems_desc'] = 'Additional items can be added to the app\'s m
 
 Link-opening methods are: app (for linking to an activity supported by the app), inappbrowser (for opening a link in a browser without leaving the app), browser (for opening the link in the device default browser outside the app) and embedded (for displaying the link in an iframe in a new page in the app).
 
+When items are missing a translation for a given language, they will use other languages as fallback unless "_only" is appended to the language code.
+
 For example:
 <pre>
 App help|https://someurl.xyz/help|inappbrowser
 My grades|https://someurl.xyz/local/mygrades/index.php|embedded|en
 Mis calificaciones|https://someurl.xyz/local/mygrades/index.php|embedded|es
+You will only see this in English|https://someurl.xyz/english|browser|en_only
 </pre>';
+$string['darkmode'] = 'Dark mode';
 $string['disabledfeatures'] = 'Disabled features';
 $string['disabledfeatures_desc'] = 'Select here the features you want to disable in the Mobile app for your site. Please note that some features listed here could be already disabled via other site settings. You will have to log out and log in again in the app to see the changes.';
 $string['displayerrorswarning'] = 'Display debug messages (debugdisplay) is enabled. It should be disabled.';
@@ -84,6 +88,8 @@ $string['mobilecssurl'] = 'CSS';
 $string['mobilefeatures'] = 'Mobile features';
 $string['mobilenotificationsdisabledwarning'] = 'Mobile notifications are not enabled. They should be enabled in Notification settings.';
 $string['mobilesettings'] = 'Mobile settings';
+$string['moodleappsportalfeatureswarning'] = 'Please note that some features may be restricted depending on your Moodle app subscription. For details, visit the <a href="{$a}" target="_blank">Moodle Apps Portal</a>.';
+$string['oauth2identityproviders'] = 'OAuth 2 identity providers';
 $string['offlineuse'] = 'Offline use';
 $string['pluginname'] = 'Moodle app tools';
 $string['pluginnotenabledorconfigured'] = 'Plugin not enabled or configured.';
@@ -91,7 +97,7 @@ $string['readingthisemailgettheapp'] = 'Reading this in an email? <a href="{$a}"
 $string['remoteaddons'] = 'Remote add-ons';
 $string['selfsignedoruntrustedcertificatewarning'] = 'It seems that the HTTPS certificate is self-signed or not trusted. The mobile app will only work with trusted sites.';
 $string['setuplink'] = 'App download page';
-$string['setuplink_desc'] = 'URL of page with links to download the mobile app from the App Store and Google Play.';
+$string['setuplink_desc'] = 'URL of page with options to download the mobile app from the App Store and Google Play. The app download page link is displayed in the page footer and in a user\'s profile. Leave blank to not display a link.';
 $string['smartappbanners'] = 'App Banners';
 $string['typeoflogin'] = 'Type of login';
 $string['typeoflogin_desc'] = 'If the site uses a SSO authentication method, then select via a browser window or via an embedded browser. An embedded browser provides a better user experience, though it doesn\'t work with all SSO plugins.';

admin/tool/mobile/launch.php

@@ -30,10 +30,16 @@ require_once($CFG->libdir . '/externallib.php');
 
 $serviceshortname  = required_param('service',  PARAM_ALPHANUMEXT);
 $passport          = required_param('passport',  PARAM_RAW);    // Passport send from the app to validate the response URL.
-$urlscheme         = optional_param('urlscheme', 'moodlemobile', PARAM_ALPHANUM); // The URL scheme the app supports.
+$urlscheme         = optional_param('urlscheme', 'moodlemobile', PARAM_NOTAGS); // The URL scheme the app supports.
 $confirmed         = optional_param('confirmed', false, PARAM_BOOL);  // If we are being redirected after user confirmation.
 $oauthsso          = optional_param('oauthsso', 0, PARAM_INT); // Id of the OpenID issuer (for OAuth direct SSO).
 
+// Validate that the urlscheme is valid.
+if (!preg_match('/^[a-zA-Z][a-zA-Z0-9-\+\.]*$/', $urlscheme)) {
+    throw new moodle_exception('Invalid parameter: the value of urlscheme isn\'t valid. ' .
+            'It should start with a letter and can only contain letters, numbers and the characters "." "+" "-".');
+}
+
 // Check web services enabled.
 if (!$CFG->enablewebservices) {
     throw new moodle_exception('enablewsdescription', 'webservice');

admin/tool/mobile/settings.php

@@ -50,6 +50,12 @@ if ($hassiteconfig) {
 
     // Show only mobile settings if the mobile service is enabled.
     if (!empty($CFG->enablemobilewebservice)) {
+        // General notification about limited features due to app restrictions.
+        $notify = new \core\output\notification(
+            get_string('moodleappsportalfeatureswarning', 'tool_mobile', tool_mobile\api::MOODLE_APPS_PORTAL_URL),
+            \core\output\notification::NOTIFY_WARNING);
+        $featuresnotice = $OUTPUT->render($notify);
+
         // Type of login.
         $temp = new admin_settingpage('mobileauthentication', new lang_string('mobileauthentication', 'tool_mobile'));
         $options = array(
@@ -63,13 +69,15 @@ if ($hassiteconfig) {
 
         $temp->add(new admin_setting_configtext('tool_mobile/forcedurlscheme',
                     new lang_string('forcedurlscheme_key', 'tool_mobile'),
-                    new lang_string('forcedurlscheme', 'tool_mobile'), '', PARAM_ALPHANUM));
+                    new lang_string('forcedurlscheme', 'tool_mobile'), '', PARAM_NOTAGS));
 
         $ADMIN->add('mobileapp', $temp);
 
         // Appearance related settings.
         $temp = new admin_settingpage('mobileappearance', new lang_string('mobileappearance', 'tool_mobile'));
 
+        $temp->add(new admin_setting_heading('tool_mobile/moodleappsportalfeaturesappearance', '', $featuresnotice));
+
         $temp->add(new admin_setting_configtext('mobilecssurl', new lang_string('mobilecssurl', 'tool_mobile'),
                     new lang_string('configmobilecssurl', 'tool_mobile'), '', PARAM_URL));
 
@@ -102,6 +110,8 @@ if ($hassiteconfig) {
         // Features related settings.
         $temp = new admin_settingpage('mobilefeatures', new lang_string('mobilefeatures', 'tool_mobile'));
 
+        $temp->add(new admin_setting_heading('tool_mobile/moodleappsportalfeatures', '', $featuresnotice));
+
         $temp->add(new admin_setting_heading('tool_mobile/logout',
                     new lang_string('logout'), ''));
 

admin/tool/mobile/tests/externallib_test.php

@@ -99,6 +99,7 @@ class tool_mobile_external_testcase extends externallib_advanced_testcase {
         );
         $this->assertEquals($expected, $result);
 
+        $this->setAdminUser();
         // Change some values.
         set_config('registerauth', 'email');
         $authinstructions = 'Something with <b>html tags</b>';
@@ -112,6 +113,18 @@ class tool_mobile_external_testcase extends externallib_advanced_testcase {
         set_config('lang', 'a_b');  // Set invalid lang.
         set_config('disabledfeatures', 'myoverview', 'tool_mobile');
 
+        // Enable couple of issuers.
+        $issuer = \core\oauth2\api::create_standard_issuer('google');
+        $irecord = $issuer->to_record();
+        $irecord->clientid = 'mock';
+        $irecord->clientsecret = 'mock';
+        core\oauth2\api::update_issuer($irecord);
+
+        set_config('hostname', 'localhost', 'auth_cas');
+        set_config('auth_logo', 'http://invalidurl.com//invalid/', 'auth_cas');
+        set_config('auth_name', 'CAS', 'auth_cas');
+        set_config('auth', 'oauth2,cas');
+
         list($authinstructions, $notusedformat) = external_format_text($authinstructions, FORMAT_MOODLE, $context->id);
         $expected['registerauth'] = 'email';
         $expected['authinstructions'] = $authinstructions;
@@ -133,7 +146,26 @@ class tool_mobile_external_testcase extends externallib_advanced_testcase {
 
         $result = external::get_public_config();
         $result = external_api::clean_returnvalue(external::get_public_config_returns(), $result);
+        // First check providers.
+        $identityproviders = $result['identityproviders'];
+        unset($result['identityproviders']);
+
+        $this->assertEquals('Google', $identityproviders[0]['name']);
+        $this->assertEquals($irecord->image, $identityproviders[0]['iconurl']);
+        $this->assertContains($CFG->wwwroot, $identityproviders[0]['url']);
+
+        $this->assertEquals('CAS', $identityproviders[1]['name']);
+        $this->assertEmpty($identityproviders[1]['iconurl']);
+        $this->assertContains($CFG->wwwroot, $identityproviders[1]['url']);
+
         $this->assertEquals($expected, $result);
+
+        // Change providers img.
+        $newurl = 'validimage.png';
+        set_config('auth_logo', $newurl, 'auth_cas');
+        $result = external::get_public_config();
+        $result = external_api::clean_returnvalue(external::get_public_config_returns(), $result);
+        $this->assertContains($newurl, $result['identityproviders'][1]['iconurl']);
     }
 
     /**

admin/tool/monitor/tests/privacy_test.php

@@ -28,6 +28,7 @@ defined('MOODLE_INTERNAL') || die();
 use \tool_monitor\privacy\provider;
 use \core_privacy\local\request\approved_contextlist;
 use \core_privacy\local\request\approved_userlist;
+use \core_privacy\tests\provider_testcase;
 
 /**
  * Privacy test for the event monitor
@@ -37,7 +38,7 @@ use \core_privacy\local\request\approved_userlist;
  * @copyright  2018 Adrian Greeve <adriangreeve.com>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class tool_monitor_privacy_testcase extends advanced_testcase {
+class tool_monitor_privacy_testcase extends provider_testcase {
 
     /**
      * Set up method.

admin/tool/oauth2/classes/form/issuer.php

@@ -46,6 +46,9 @@ class issuer extends persistent {
     /** @var string $type */
     protected $type;
 
+    /** @var boolean $showrequireconfirm Whether to show the require confirmation email checkbox or not. */
+    protected $showrequireconfirm;
+
     /**
      * Constructor.
      *
@@ -71,6 +74,7 @@ class issuer extends persistent {
         if (array_key_exists('type', $customdata)) {
             $this->type = $customdata['type'];
         }
+        $this->showrequireconfirm = !empty($customdata['showrequireconfirm']);
         parent::__construct($action, $customdata, $method, $target, $attributes, $editable, $ajaxformdata);
     }
 
@@ -157,9 +161,11 @@ class issuer extends persistent {
         $mform->addElement('checkbox', 'showonloginpage', get_string('issuershowonloginpage', 'tool_oauth2'));
         $mform->addHelpButton('showonloginpage', 'issuershowonloginpage', 'tool_oauth2');
 
-        // Require confirmation email for new accounts.
-        $mform->addElement('advcheckbox', 'requireconfirmation', get_string('issuerrequireconfirmation', 'tool_oauth2'));
-        $mform->addHelpButton('requireconfirmation', 'issuerrequireconfirmation', 'tool_oauth2');
+        if ($this->showrequireconfirm) {
+            // Require confirmation email for new accounts.
+            $mform->addElement('advcheckbox', 'requireconfirmation', get_string('issuerrequireconfirmation', 'tool_oauth2'));
+            $mform->addHelpButton('requireconfirmation', 'issuerrequireconfirmation', 'tool_oauth2');
+        }
 
         $mform->addElement('hidden', 'sortorder');
         $mform->setType('sortorder', PARAM_INT);

admin/tool/oauth2/issuers.php

@@ -58,7 +58,24 @@ if ($action == 'edit') {
         $PAGE->navbar->add(get_string('createnewissuer', 'tool_oauth2'));
     }
 
-    $mform = new \tool_oauth2\form\issuer(null, ['persistent' => $issuer]);
+    $showrequireconfirm = false;
+    if (!empty($issuerid)) {
+        // Show the "Require confirmation email" checkbox for trusted issuers like Google, Facebook and Microsoft.
+        $likefacebook = $DB->sql_like('url', ':facebook');
+        $likegoogle = $DB->sql_like('url', ':google');
+        $likemicrosoft = $DB->sql_like('url', ':microsoft');
+        $params = [
+            'issuerid' => $issuerid,
+            'facebook' => '%facebook%',
+            'google' => '%google%',
+            'microsoft' => '%microsoft%',
+        ];
+        $select = "issuerid = :issuerid AND ($likefacebook OR $likegoogle OR $likemicrosoft)";
+        // We're querying from the oauth2_endpoint table because the base URLs of FB and Microsoft can be empty in the issuer table.
+        $showrequireconfirm = $DB->record_exists_select('oauth2_endpoint', $select, $params);
+    }
+
+    $mform = new \tool_oauth2\form\issuer(null, ['persistent' => $issuer, 'showrequireconfirm' => $showrequireconfirm]);
 }
 
 if ($mform && $mform->is_cancelled()) {
@@ -108,9 +125,11 @@ if ($mform && $mform->is_cancelled()) {
 
     $type = required_param('type', PARAM_ALPHA);
     $docs = required_param('docslink', PARAM_ALPHAEXT);
+    $showrequireconfirm = optional_param('showrequireconfirm', false, PARAM_BOOL);
     require_sesskey();
     $issuer = core\oauth2\api::init_standard_issuer($type);
-    $mform = new \tool_oauth2\form\issuer(null, ['persistent' => $issuer, 'type' => $type]);
+    $mform = new \tool_oauth2\form\issuer(null, ['persistent' => $issuer, 'type' => $type,
+        'showrequireconfirm' => $showrequireconfirm]);
 
     echo $OUTPUT->header();
     $mform->display();
@@ -178,26 +197,37 @@ if ($mform && $mform->is_cancelled()) {
     $issuers = core\oauth2\api::get_all_issuers();
     echo $renderer->issuers_table($issuers);
 
+    // Google template.
     $docs = 'admin/tool/oauth2/issuers/google';
-    $params = ['action' => 'edittemplate', 'type' => 'google', 'sesskey' => sesskey(), 'docslink' => $docs];
+    $params = ['action' => 'edittemplate', 'type' => 'google', 'sesskey' => sesskey(), 'docslink' => $docs,
+        'showrequireconfirm' => true];
     $addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
     echo $renderer->single_button($addurl, get_string('createnewgoogleissuer', 'tool_oauth2'));
+
+    // Microsoft template.
     $docs = 'admin/tool/oauth2/issuers/microsoft';
-    $params = ['action' => 'edittemplate', 'type' => 'microsoft', 'sesskey' => sesskey(), 'docslink' => $docs];
+    $params = ['action' => 'edittemplate', 'type' => 'microsoft', 'sesskey' => sesskey(), 'docslink' => $docs,
+        'showrequireconfirm' => true];
     $addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
     echo $renderer->single_button($addurl, get_string('createnewmicrosoftissuer', 'tool_oauth2'));
+
+    // Facebook template.
     $docs = 'admin/tool/oauth2/issuers/facebook';
-    $params = ['action' => 'edittemplate', 'type' => 'microsoft', 'sesskey' => sesskey(), 'docslink' => $docs];
-    $params = ['action' => 'edittemplate', 'type' => 'facebook', 'sesskey' => sesskey(), 'docslink' => $docs];
+    $params = ['action' => 'edittemplate', 'type' => 'facebook', 'sesskey' => sesskey(), 'docslink' => $docs,
+        'showrequireconfirm' => true];
     $addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
     echo $renderer->single_button($addurl, get_string('createnewfacebookissuer', 'tool_oauth2'));
-    $addurl = new moodle_url('/admin/tool/oauth2/issuers.php', ['action' => 'edit']);
+
+    // Nextcloud template.
     $docs = 'admin/tool/oauth2/issuers/nextcloud';
     $params = ['action' => 'edittemplate', 'type' => 'nextcloud', 'sesskey' => sesskey(), 'docslink' => $docs];
     $addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
     echo $renderer->single_button($addurl, get_string('createnewnextcloudissuer', 'tool_oauth2'));
+
+    // Generic issuer.
     $addurl = new moodle_url('/admin/tool/oauth2/issuers.php', ['action' => 'edit']);
     echo $renderer->single_button($addurl, get_string('createnewissuer', 'tool_oauth2'));
+
     echo $OUTPUT->footer();
 
 }

admin/tool/phpunit/webrunner.php

@@ -131,11 +131,12 @@ if ($execute) {
             tool_phpunit_problem('Can not create configuration file');
         }
     }
-    $configdir = escapeshellarg($configdir);
-    // no cleanup of path - this is tricky because we can not use escapeshellarg and friends for escaping,
-    // this is from admin user so PARAM_PATH must be enough
     chdir($CFG->dirroot);
-    passthru("php $CFG->admin/tool/phpunit/cli/util.php --run -c $configdir $testclass $testpath", $code);
+    $configdir = escapeshellarg($configdir);
+    $cleanclass = escapeshellarg($testclass);
+    $cleanpath = escapeshellarg($testpath);
+
+    passthru("php $CFG->admin/tool/phpunit/cli/util.php --run -c $configdir $cleanclass $cleanpath", $code);
     chdir($oldcwd);
 
     echo '</pre>';
@@ -146,7 +147,7 @@ if ($execute) {
 }
 
 echo $OUTPUT->box_start('generalbox boxwidthwide boxaligncenter');
-echo '<form method="get" action="webrunner.php">';
+echo '<form method="post" action="webrunner.php">';
 echo '<fieldset class="invisiblefieldset">';
 echo '<label for="testpath">Test one file</label> ';
 echo '<input type="text" id="testpath" name="testpath" value="'.s($testpath).'" size="50" /> (all test cases from webrunner.xml if empty)';

admin/tool/policy/user.php

@@ -39,15 +39,29 @@ if ($userid != $USER->id) {
     if (!has_capability('tool/policy:acceptbehalf', $context)) {
         require_capability('tool/policy:viewacceptances', $context);
     }
+
+    $user = core_user::get_user($userid);
+    $PAGE->navigation->extend_for_user($user);
 }
 
+$title = get_string('policiesagreements', 'tool_policy');
+
 $PAGE->set_context($context);
 $PAGE->set_pagelayout('standard');
 $PAGE->set_url(new moodle_url('/admin/tool/policy/user.php', ['userid' => $userid]));
+$PAGE->set_title($title);
+
+if ($userid == $USER->id &&
+        ($profilenode = $PAGE->settingsnav->find('myprofile', null))) {
+
+    $profilenode->make_active();
+}
+
+$PAGE->navbar->add($title);
 
 $output = $PAGE->get_renderer('tool_policy');
 echo $output->header();
-echo $output->heading(get_string('policiesagreements', 'tool_policy'));
+echo $output->heading($title);
 $acceptances = new \tool_policy\output\acceptances($userid, $returnurl);
 echo $output->render($acceptances);
 $PAGE->requires->js_call_amd('tool_policy/acceptmodal', 'getInstance', [context_system::instance()->id]);

admin/tool/recyclebin/tests/behat/basic_functionality.feature

@@ -127,3 +127,36 @@ Feature: Basic recycle bin functionality
     And I press "Yes"
     And I should see "Recycle bin has been emptied"
     And I should see "There are no items in the recycle bin."
+
+  @javascript
+  Scenario: Show recycle bin on category action menu
+    Given I log in as "admin"
+    And I navigate to "Courses >  Manage courses and categories" in site administration
+    And I click on "Actions menu" "link"
+    And I click on "Recycle bin" "link"
+    Then I should see "There are no items in the recycle bin."
+
+  @javascript
+  Scenario: Not show recycle bin empty on category action menu whit autohide enable
+    Given I log in as "admin"
+    And the following config values are set as admin:
+      | categorybinenable | 0 | tool_recyclebin |
+    And I navigate to "Courses >  Manage courses and categories" in site administration
+    And I click on "Actions menu" "link"
+    Then I should not see "Recycle bin"
+
+  @javascript
+  Scenario: Show recycle bin not empty on category action menu whit autohide enable
+    Given I log in as "admin"
+    And the following config values are set as admin:
+      | autohide | 1 | tool_recyclebin |
+    And I navigate to "Courses >  Manage courses and categories" in site administration
+    And I click on "Actions menu" "link"
+    Then I should not see "Recycle bin"
+    And I click on "delete" action for "Course 2" in management course listing
+    And I press "Delete"
+    And I should see "Deleting C2"
+    And I should see "C2 has been completely deleted"
+    And I press "Continue"
+    When I click on "Actions menu" "link"
+    Then I should see "Recycle bin"

admin/tool/unsuproles/index.php

@@ -100,9 +100,9 @@ if (!$problems) {
         $count = $problem->racount;
         $edit = array();
         $aurl = new moodle_url('/admin/roles/define.php', array('roleid'=>$problem->roleid, 'action'=>'edit'));
-        $edit[] = html_writer::link($aurl, $OUTPUT->pix_icon('t/edit', 'core', get_string('edit')));
+        $edit[] = html_writer::link($aurl, $OUTPUT->pix_icon('t/edit', get_string('edit')));
         $aurl = new moodle_url($PAGE->url, array('roleid'=>$problem->roleid, 'contextlevel'=>$problem->contextlevel, 'action'=>'delete'));
-        $edit[] = html_writer::link($aurl, $OUTPUT->pix_icon('t/delete', 'core', get_string('delete')));
+        $edit[] = html_writer::link($aurl, $OUTPUT->pix_icon('t/delete', get_string('delete')));
         $data[] = array($levelname, $rolename, $count, implode(' ', $edit));
     }
     $table = new html_table();

admin/tool/uploadcourse/classes/course.php

@@ -412,6 +412,12 @@ class tool_uploadcourse_course {
                 $this->error('invalidshortname', new lang_string('invalidshortname', 'tool_uploadcourse'));
                 return false;
             }
+
+            // Ensure we don't overflow the maximum length of the shortname field.
+            if (core_text::strlen($this->shortname) > 255) {
+                $this->error('invalidshortnametoolong', new lang_string('invalidshortnametoolong', 'tool_uploadcourse', 255));
+                return false;
+            }
         }
 
         $exists = $this->exists();
@@ -479,6 +485,12 @@ class tool_uploadcourse_course {
             return false;
         }
 
+        // Ensure we don't overflow the maximum length of the fullname field.
+        if (!empty($coursedata['fullname']) && core_text::strlen($coursedata['fullname']) > 254) {
+            $this->error('invalidfullnametoolong', new lang_string('invalidfullnametoolong', 'tool_uploadcourse', 254));
+            return false;
+        }
+
         // If the course does not exist, or will be forced created.
         if (!$exists || $mode === tool_uploadcourse_processor::MODE_CREATE_ALL) {
 
@@ -720,6 +732,12 @@ class tool_uploadcourse_course {
             }
         }
 
+        // Visibility can only be 0 or 1.
+        if (!empty($coursedata['visible']) AND !($coursedata['visible'] == 0 OR $coursedata['visible'] == 1)) {
+            $this->error('invalidvisibilitymode', new lang_string('invalidvisibilitymode', 'tool_uploadcourse'));
+            return false;
+        }
+
         // Saving data.
         $this->data = $coursedata;
         $this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata);

admin/tool/uploadcourse/lang/en/tool_uploadcourse.php

@@ -90,8 +90,11 @@ $string['invalidcsvfile'] = 'Invalid input CSV file';
 $string['invalidencoding'] = 'Invalid encoding';
 $string['invalidmode'] = 'Invalid mode selected';
 $string['invalideupdatemode'] = 'Invalid update mode selected';
+$string['invalidvisibilitymode'] = 'Invalid visible mode';
 $string['invalidroles'] = 'Invalid role names: {$a}';
 $string['invalidshortname'] = 'Invalid shortname';
+$string['invalidfullnametoolong'] = 'The fullname field is limited to {$a} characters';
+$string['invalidshortnametoolong'] = 'The shortname field is limited to {$a} characters';
 $string['missingmandatoryfields'] = 'Missing value for mandatory fields: {$a}';
 $string['missingshortnamenotemplate'] = 'Missing shortname and shortname template not set';
 $string['mode'] = 'Upload mode';

admin/tool/uploadcourse/tests/course_test.php

@@ -82,6 +82,47 @@ class tool_uploadcourse_course_testcase extends advanced_testcase {
         $this->assertArrayHasKey('invalidshortname', $co->get_errors());
     }
 
+    public function test_invalid_shortname_too_long() {
+        $this->resetAfterTest();
+
+        $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
+        $updatemode = tool_uploadcourse_processor::UPDATE_NOTHING;
+
+        $upload = new tool_uploadcourse_course($mode, $updatemode, [
+            'category' => 1,
+            'fullname' => 'New course',
+            'shortname' => str_repeat('X', 2000),
+        ]);
+
+        $this->assertFalse($upload->prepare());
+        $this->assertArrayHasKey('invalidshortnametoolong', $upload->get_errors());
+    }
+
+    public function test_invalid_fullname_too_long() {
+        $this->resetAfterTest();
+
+        $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
+        $updatemode = tool_uploadcourse_processor::UPDATE_NOTHING;
+
+        $upload = new tool_uploadcourse_course($mode, $updatemode, [
+            'category' => 1,
+            'fullname' => str_repeat('X', 2000),
+        ]);
+
+        $this->assertFalse($upload->prepare());
+        $this->assertArrayHasKey('invalidfullnametoolong', $upload->get_errors());
+    }
+
+    public function test_invalid_visibility() {
+        $this->resetAfterTest(true);
+        $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
+        $updatemode = tool_uploadcourse_processor::UPDATE_NOTHING;
+        $data = array('shortname' => 'test', 'fullname' => 'New course', 'summary' => 'New', 'category' => 1, 'visible' => 2);
+        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
+        $this->assertFalse($co->prepare());
+        $this->assertArrayHasKey('invalidvisibilitymode', $co->get_errors());
+    }
+
     public function test_create() {
         global $DB;
         $this->resetAfterTest(true);

admin/tool/usertours/classes/manager.php

@@ -257,7 +257,7 @@ class manager {
                 'title' => get_string('importtour', 'tool_usertours'),
             ],
             (object) [
-                'link'  => new \moodle_url('https://moodle.net/tours'),
+                'link'  => new \moodle_url('https://archive.moodle.net/tours'),
                 'linkproperties' => [
                         'target' => '_blank',
                     ],
@@ -720,6 +720,13 @@ class manager {
      * @param   int     $direction
      */
     protected static function _move_tour(tour $tour, $direction) {
+        // We can't move the first tour higher, nor the last tour any lower.
+        if (($tour->is_first_tour() && $direction == helper::MOVE_UP) ||
+                ($tour->is_last_tour() && $direction == helper::MOVE_DOWN)) {
+
+            return;
+        }
+
         $currentsortorder   = $tour->get_sortorder();
         $targetsortorder    = $currentsortorder + $direction;
 
@@ -848,6 +855,9 @@ class manager {
         }
         $existingtourrecords->close();
 
+        // Ensure we correct the sortorder in any existing tours, prior to adding latest shipped tours.
+        helper::reset_tour_sortorder();
+
         foreach (array_reverse($shippedtours) as $filename => $version) {
             $filepath = $CFG->dirroot . "/{$CFG->admin}/tool/usertours/tours/" . $filename;
             $tourjson = file_get_contents($filepath);

admin/tool/usertours/db/upgrade.php

@@ -64,5 +64,12 @@ function xmldb_tool_usertours_upgrade($oldversion) {
     // Automatically generated Moodle v3.7.0 release upgrade line.
     // Put any upgrade step following this.
 
+    if ($oldversion < 2019052001) {
+        // Updating shipped tours will fix broken sortorder records in existing tours.
+        manager::update_shipped_tours();
+
+        upgrade_plugin_savepoint(true, 2019052001, 'tool', 'usertours');
+    }
+
     return true;
 }

admin/tool/usertours/tests/manager_test.php

@@ -122,6 +122,83 @@ class tool_usertours_manager_testcase extends advanced_testcase {
         $rcm->invokeArgs($manager, $arguments);
     }
 
+    /**
+     * Data provider for test_move_tour
+     *
+     * @return array
+     */
+    public function move_tour_provider() {
+        $alltours = [
+            ['name' => 'Tour 1'],
+            ['name' => 'Tour 2'],
+            ['name' => 'Tour 3'],
+        ];
+
+        return [
+            'Move up' => [
+                $alltours,
+                'Tour 2',
+                \tool_usertours\helper::MOVE_UP,
+                0,
+            ],
+            'Move down' => [
+                $alltours,
+                'Tour 2',
+                \tool_usertours\helper::MOVE_DOWN,
+                2,
+            ],
+            'Move up (first)' => [
+                $alltours,
+                'Tour 1',
+                \tool_usertours\helper::MOVE_UP,
+                0,
+            ],
+            'Move down (last)' => [
+                $alltours,
+                'Tour 3',
+                \tool_usertours\helper::MOVE_DOWN,
+                2,
+            ],
+        ];
+    }
+
+    /**
+     * Test moving tours (changing sortorder)
+     *
+     * @dataProvider move_tour_provider
+     *
+     * @param array $alltours
+     * @param string $movetourname
+     * @param int $direction
+     * @param int $expectedsortorder
+     * @return void
+     */
+    public function test_move_tour($alltours, $movetourname, $direction, $expectedsortorder) {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Clear out existing tours so ours are the only ones, otherwise we can't predict the sortorder.
+        $DB->delete_records('tool_usertours_tours');
+
+        foreach ($alltours as $tourconfig) {
+            $this->helper_create_tour((object) $tourconfig);
+        }
+
+        // Load our tour to move.
+        $record = $DB->get_record('tool_usertours_tours', ['name' => $movetourname]);
+        $tour = \tool_usertours\tour::load_from_record($record);
+
+        // Call protected method via reflection.
+        $class = new ReflectionClass(\tool_usertours\manager::class);
+        $method = $class->getMethod('_move_tour');
+        $method->setAccessible(true);
+        $method->invokeArgs(null, [$tour, $direction]);
+
+        // Assert expected sortorder.
+        $this->assertEquals($expectedsortorder, $tour->get_sortorder());
+    }
+
     /**
      * Data Provider for get_matching_tours tests.
      *

admin/tool/usertours/version.php

@@ -24,6 +24,6 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2019052000;            // The current module version (Date: YYYYMMDDXX).
+$plugin->version   = 2019052001;            // The current module version (Date: YYYYMMDDXX).
 $plugin->requires  = 2019051100;            // Requires this Moodle version.
 $plugin->component = 'tool_usertours';      // Full name of the plugin (used for diagnostics).

admin/tool/xmldb/actions/view_xml/view_xml.class.php

@@ -68,13 +68,13 @@ class view_xml extends XMLDBAction {
 
         // Get the file parameter
         $file = required_param('file', PARAM_PATH);
-        $file = $CFG->dirroot . $file;
-        // File must be under $CFG->wwwroot and
-        // under one db directory (simple protection)
-        if (substr($file, 0, strlen($CFG->dirroot)) == $CFG->dirroot &&
-            substr(dirname($file), -2, 2) == 'db') {
+
+        $fullpath = $CFG->dirroot . $file;
+        // File param must start with / and end with /db/install.xml to be safe.
+        if (substr($file, 0, 1) == '/' &&
+            substr($file, -15, 15) == '/db/install.xml') {
             // Everything is ok. Load the file to memory
-            $this->output = file_get_contents($file);
+            $this->output = file_get_contents($fullpath);
         } else {
             // Switch to HTML and error
             $this->does_generate = ACTION_GENERATE_HTML;

admin/user.php

@@ -401,7 +401,7 @@
             $row = array ();
             $row[] = "<a href=\"../user/view.php?id=$user->id&amp;course=$site->id\">$fullname</a>";
             foreach ($extracolumns as $field) {
-                $row[] = $user->{$field};
+                $row[] = s($user->{$field});
             }
             $row[] = $user->city;
             $row[] = $user->country;