Moodle release 3.7.4

Co-authored-by: Paul Holden <paulh@moodle.com>

.eslintrc

@@ -203,7 +203,7 @@
       }
     },
     {
-      files: ["**/amd/src/*.js"],
+      files: ["**/amd/src/*.js", "Gruntfile.js"],
       // Check AMD with some slightly stricter rules.
       rules: {
         'no-unused-vars': 'error',

.jshintignore

@@ -0,0 +1,2 @@
+**/amd/**
+/*.js

.nvmrc

@@ -0,0 +1 @@
+v8.16.1

.travis.yml

@@ -11,7 +11,10 @@ notifications:
 
 language: php
 
-dist: trusty
+dist: xenial
+
+services:
+    - mysql
 
 php:
     # We only run the highest and lowest supported versions to reduce the load on travis-ci.org.
@@ -20,10 +23,6 @@ php:
 
 addons:
   postgresql: "9.6"
-  packages:
-    - mysql-server-5.6
-    - mysql-client-core-5.6
-    - mysql-client-5.6
 
 env:
     # Although we want to run these jobs and see failures as quickly as possible, we also want to get the slowest job to
@@ -71,10 +70,10 @@ install:
         then
             sudo mkdir /mnt/ramdisk
             sudo mount -t tmpfs -o size=1024m tmpfs /mnt/ramdisk
-            sudo stop mysql
+            sudo service mysql stop
             sudo mv /var/lib/mysql /mnt/ramdisk
             sudo ln -s /mnt/ramdisk/mysql /var/lib/mysql
-            sudo start mysql
+            sudo service mysql restart
         fi
     - >
         if [ "$DB" = 'pgsql' ];

Gruntfile.js

@@ -24,6 +24,7 @@
  * Grunt configuration
  */
 
+/* eslint-env node */
 module.exports = function(grunt) {
     var path = require('path'),
         tasks = {},

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/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 (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.
+                    $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

@@ -46,7 +46,7 @@ $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
+    // 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

@@ -40,7 +40,7 @@ $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
+    // 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

@@ -1469,9 +1469,11 @@ class core_admin_renderer extends plugin_renderer_base {
                     $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();
@@ -2110,6 +2112,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'));
     }
 
@@ -623,7 +625,7 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi
             echo "</label>\n";
         }
         if ($helpicon) {
-            echo '<span class="pull-xs-right text-nowrap">'.$helpicon.'</span>';
+            echo '<span class="float-sm-right text-nowrap">'.$helpicon.'</span>';
         }
         echo '</div>';
         if (isset($this->errors[$name])) {

admin/settings/subsystems.php

@@ -48,7 +48,4 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page
 
     $optionalsubsystems->add(new admin_setting_configcheckbox('allowstealth', new lang_string('allowstealthmodules'),
         new lang_string('allowstealthmodules_help'), 0, 1, 0));
-
-    $optionalsubsystems->add(new admin_setting_configcheckbox('enablecoursepublishing',
-        new lang_string('enablecoursepublishing', 'hub'), new lang_string('enablecoursepublishing_help', 'hub'), 0));
 }

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) {

admin/testoutgoingmailconf.php

@@ -52,7 +52,7 @@ if ($data) {
     // 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 +66,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/tests/behat/manage_tokens.feature

@@ -9,6 +9,7 @@ Feature: Manage tokens
     | username  | password  | firstname | lastname |
     | testuser  | testuser  | Joe | Bloggs |
     | testuser2 | testuser2 | TestFirstname | TestLastname |
+    And I change window size to "small"
     And I log in as "admin"
     And I am on site homepage
 

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/tests/behat/edit_permissions.feature

@@ -20,12 +20,12 @@ Feature: Edit capabilities
     And I set the following system permissions of "Teacher" role:
       | capability | permission |
       | block/mnet_hosts:myaddinstance | Allow |
-      | moodle/community:add | Inherit |
+      | moodle/site:messageanyuser | Inherit |
       | moodle/grade:managesharedforms | Prevent |
       | moodle/course:request | Prohibit |
     When I follow "Edit Teacher role"
     Then "block/mnet_hosts:myaddinstance" capability has "Allow" permission
-    And "moodle/community:add" capability has "Not set" permission
+    And "moodle/site:messageanyuser" capability has "Not set" permission
     And "moodle/grade:managesharedforms" capability has "Prevent" permission
     And "moodle/course:request" capability has "Prohibit" permission
 

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/capability/renderer.php

@@ -114,15 +114,17 @@ class tool_capability_renderer extends plugin_renderer_base {
         }
 
         // Start the list item, and print the context name as a link to the place to make changes.
-        if ($contextid == context_system::instance()->id) {
-            $url = new moodle_url('/admin/roles/manage.php');
-            $title = get_string('changeroles', 'tool_capability');
-        } else {
-            $url = new moodle_url('/admin/roles/override.php', array('contextid' => $contextid));
-            $title = get_string('changeoverrides', 'tool_capability');
-        }
         $context = context::instance_by_id($contextid);
-        $html = $this->output->heading(html_writer::link($url, $context->get_context_name(), array('title' => $title)), 3);
+
+        if ($context instanceof context_system) {
+            $url = new moodle_url('/admin/roles/manage.php');
+        } else {
+            $url = new moodle_url('/admin/roles/permissions.php', ['contextid' => $contextid]);
+        }
+
+        $title = get_string('permissionsincontext', 'core_role', $context->get_context_name());
+
+        $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)) {
@@ -133,4 +135,4 @@ class tool_capability_renderer extends plugin_renderer_base {
         return $html;
     }
 
-}
+}

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/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/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/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/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/form/purpose.php

@@ -440,6 +440,9 @@ class purpose extends persistent {
         }
         if (!empty($data->sensitivedatareasons) && is_array($data->sensitivedatareasons)) {
             $data->sensitivedatareasons = implode(',', $data->sensitivedatareasons);
+        } else {
+            // Nothing selected. Set default value of null.
+            $data->sensitivedatareasons = null;
         }
 
         // A single value.

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/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/lp/amd/build/course_competency_settings.min.js

@@ -1 +1 @@
-define(["jquery","core/notification","tool_lp/dialogue","core/str","core/ajax","core/templates"],function(a,b,c,d,e,f){var g=function(b){a(b).on("click",this.configureSettings.bind(this))};return g.prototype._dialogue=null,g.prototype.configureSettings=function(e){var g=a(e.target).closest("a").data("courseid"),h=a(e.target).closest("a").data("pushratingstouserplans"),i={courseid:g,settings:{pushratingstouserplans:h}};e.preventDefault(),f.render("tool_lp/course_competency_settings",i).done(function(a){d.get_string("configurecoursecompetencysettings","tool_lp").done(function(b){this._dialogue=new c(b,a,this.addListeners.bind(this))}.bind(this)).fail(b.exception)}.bind(this)).fail(b.exception)},g.prototype.addListeners=function(){var a=this._find('[data-action="save"]');a.on("click",this.saveSettings.bind(this));var b=this._find('[data-action="cancel"]');b.on("click",this.cancelChanges.bind(this))},g.prototype.cancelChanges=function(a){a.preventDefault(),this._dialogue.close()},g.prototype._find=function(b){return a('[data-region="coursecompetencysettings"]').find(b)},g.prototype.saveSettings=function(a){a.preventDefault();var c=this._find('input[name="pushratingstouserplans"]:checked').val(),d=this._find('input[name="courseid"]').val(),f={pushratingstouserplans:c};e.call([{methodname:"core_competency_update_course_competency_settings",args:{courseid:d,settings:f}}])[0].done(function(){this.refreshCourseCompetenciesPage()}.bind(this)).fail(b.exception)},g.prototype.refreshCourseCompetenciesPage=function(){var c=this._find('input[name="courseid"]').val();e.call([{methodname:"tool_lp_data_for_course_competencies_page",args:{courseid:c,moduleid:0}}])[0].done(function(c){f.render("tool_lp/course_competencies_page",c).done(function(b,c){a('[data-region="coursecompetenciespage"]').replaceWith(b),f.runTemplateJS(c),this._dialogue.close()}.bind(this)).fail(b.exception)}.bind(this)).fail(b.exception)},g});
+define(["jquery","core/notification","tool_lp/dialogue","core/str","core/ajax","core/templates","core/pending"],function(a,b,c,d,e,f,g){var h=function(b){a(b).on("click",this.configureSettings.bind(this))};return h.prototype._dialogue=null,h.prototype.configureSettings=function(e){var h=new g,i=a(e.target).closest("a").data("courseid"),j=a(e.target).closest("a").data("pushratingstouserplans"),k={courseid:i,settings:{pushratingstouserplans:j}};e.preventDefault(),a.when(d.get_string("configurecoursecompetencysettings","tool_lp"),f.render("tool_lp/course_competency_settings",k)).then(function(a,b){return this._dialogue=new c(a,b[0],this.addListeners.bind(this)),this._dialogue}.bind(this)).then(h.resolve)["catch"](b.exception)},h.prototype.addListeners=function(){var a=this._find('[data-action="save"]');a.on("click",this.saveSettings.bind(this));var b=this._find('[data-action="cancel"]');b.on("click",this.cancelChanges.bind(this))},h.prototype.cancelChanges=function(a){a.preventDefault(),this._dialogue.close()},h.prototype._find=function(b){return a('[data-region="coursecompetencysettings"]').find(b)},h.prototype.saveSettings=function(a){var c=new g;a.preventDefault();var d=this._find('input[name="pushratingstouserplans"]:checked').val(),f=this._find('input[name="courseid"]').val(),h={pushratingstouserplans:d};e.call([{methodname:"core_competency_update_course_competency_settings",args:{courseid:f,settings:h}}])[0].then(function(){return this.refreshCourseCompetenciesPage()}.bind(this)).then(c.resolve)["catch"](b.exception)},h.prototype.refreshCourseCompetenciesPage=function(){var c=this._find('input[name="courseid"]').val(),d=new g;e.call([{methodname:"tool_lp_data_for_course_competencies_page",args:{courseid:c,moduleid:0}}])[0].then(function(a){return f.render("tool_lp/course_competencies_page",a)}).then(function(b,c){f.replaceNode(a('[data-region="coursecompetenciespage"]'),b,c),this._dialogue.close()}.bind(this)).then(d.resolve)["catch"](b.exception)},h});

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 this._render().then(function(a){return f.get_string("rate","tool_lp").then(function(b){this._popup=new d(b,a,this._afterRender.bind(this))}.bind(this))}.bind(this)).fail(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 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});

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.done(function(f){a.push({value:"",name:d._chooseStr});for(var g=0;g<f.length;g++){var h=f[g];a.push({value:h.id,name:h.name})}d._dialogue=new e(a),d._dialogue.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}])})}).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,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});

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

@@ -27,8 +27,9 @@ define(['jquery',
         'core/templates',
         'core/str',
         'tool_lp/competencypicker',
-        'tool_lp/dragdrop-reorder'],
-       function($, notification, ajax, templates, str, Picker, dragdrop) {
+        'tool_lp/dragdrop-reorder',
+        'core/pending'],
+       function($, notification, ajax, templates, str, Picker, dragdrop, Pending) {
 
     /**
      * Constructor
@@ -117,6 +118,7 @@ define(['jquery',
      * Pick a competency
      *
      * @method pickCompetency
+     * @return {Promise}
      */
     competencies.prototype.pickCompetency = function() {
         var self = this;
@@ -132,6 +134,7 @@ define(['jquery',
             self.pickerInstance = new Picker(self.pageContextId, false, pageContextIncludes);
             self.pickerInstance.on('save', function(e, data) {
                 var compIds = data.competencyIds;
+                var pendingPromise = new Pending();
 
                 if (self.itemtype === "course") {
                     requests = [];
@@ -181,17 +184,20 @@ define(['jquery',
                     pagerender = 'tool_lp/plan_page';
                     pageregion = 'plan-page';
                 }
-                ajax.call(requests)[requests.length - 1].then(function(context) {
+                ajax.call(requests)[requests.length - 1]
+                .then(function(context) {
                     return templates.render(pagerender, context);
-                }).then(function(html, js) {
-                    $('[data-region="' + pageregion + '"]').replaceWith(html);
-                    templates.runTemplateJS(js);
+                })
+                .then(function(html, js) {
+                    templates.replaceNode($('[data-region="' + pageregion + '"]'), html, js);
                     return;
-                }).catch(notification.exception);
+                })
+                .then(pendingPromise.resolve)
+                .catch(notification.exception);
             });
         }
 
-        self.pickerInstance.display();
+        return self.pickerInstance.display();
     };
 
     /**
@@ -302,6 +308,7 @@ define(['jquery',
         if (localthis.itemtype == 'course') {
             // Course completion rule handling.
             $('[data-region="coursecompetenciespage"]').on('change', 'select[data-field="ruleoutcome"]', function(e) {
+                var pendingPromise = new Pending();
                 var requests = [];
                 var pagerender = 'tool_lp/course_competencies_page';
                 var pageregion = 'coursecompetenciespage';
@@ -314,18 +321,24 @@ define(['jquery',
                       args: {courseid: localthis.itemid, moduleid: 0}}
                 ]);
 
-                requests[1].done(function(context) {
-                    templates.render(pagerender, context).done(function(html, js) {
-                        $('[data-region="' + pageregion + '"]').replaceWith(html);
-                        templates.runTemplateJS(js);
-                    }).fail(notification.exception);
-                }).fail(notification.exception);
+                requests[1].then(function(context) {
+                    return templates.render(pagerender, context);
+                })
+                .then(function(html, js) {
+                    return templates.replaceNode($('[data-region="' + pageregion + '"]'), html, js);
+                })
+                .then(pendingPromise.resolve)
+                .catch(notification.exception);
             });
         }
 
         $('[data-region="actions"] button').click(function(e) {
+            var pendingPromise = new Pending();
             e.preventDefault();
-            localthis.pickCompetency();
+
+            localthis.pickCompetency()
+                .then(pendingPromise.resolve)
+                .catch();
         });
         $('[data-action="delete-competency-link"]').click(function(e) {
             e.preventDefault();

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

@@ -33,8 +33,12 @@ define(['jquery',
         'tool_lp/menubar',
         'tool_lp/competencypicker',
         'tool_lp/competency_outcomes',
-        'tool_lp/competencyruleconfig'],
-       function($, url, templates, notification, str, ajax, dragdrop, Ariatree, Dialogue, menubar, Picker, Outcomes, RuleConfig) {
+        'tool_lp/competencyruleconfig',
+        'core/pending',
+        ],
+       function(
+            $, url, templates, notification, str, ajax, dragdrop, Ariatree, Dialogue, menubar, Picker, Outcomes, RuleConfig, Pending
+        ) {
 
     // Private variables and functions.
     /** @var {Object} treeModel - This is an object representing the nodes in the tree. */
@@ -412,6 +416,7 @@ define(['jquery',
         if (!pickerInstance) {
             pickerInstance = new Picker(pageContextId, relatedTarget.competencyframeworkid);
             pickerInstance.on('save', function(e, data) {
+                var pendingPromise = new Pending();
                 var compIds = data.competencyIds;
 
                 var calls = [];
@@ -436,7 +441,9 @@ define(['jquery',
                     templates.runTemplateJS(js);
                     updatedRelatedCompetencies();
                     return;
-                }).catch(notification.exception);
+                })
+                .then(pendingPromise.resolve)
+                .catch(notification.exception);
             });
         }
 

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

@@ -31,8 +31,10 @@ define(['jquery',
         'core/templates',
         'tool_lp/dialogue',
         'core/str',
-        'tool_lp/tree'],
-        function($, Notification, Ajax, Templates, Dialogue, Str, Tree) {
+        'tool_lp/tree',
+        'core/pending'
+        ],
+        function($, Notification, Ajax, Templates, Dialogue, Str, Tree, Pending) {
 
     /**
      * Competency picker class.
@@ -157,6 +159,7 @@ define(['jquery',
         // Add listener for add.
         self._find('[data-region="competencylinktree"] [data-action="add"]').click(function(e) {
             e.preventDefault();
+            var pendingPromise = new Pending();
             if (!self._selectedCompetencies.length) {
                 return;
             }
@@ -168,7 +171,10 @@ define(['jquery',
                 self._trigger('save', {competencyId: self._selectedCompetencies[0]});
             }
 
+            // The dialogue here is a YUI dialogue and doesn't support Promises at all.
+            // However, it is typically synchronous so this shoudl suffice.
             self.close();
+            pendingPromise.resolve();
         });
 
         // The list of selected competencies will be modified while looping (because of the listeners above).

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

@@ -26,8 +26,10 @@ define(['jquery',
         'tool_lp/dialogue',
         'core/str',
         'core/ajax',
-        'core/templates'],
-       function($, notification, Dialogue, str, ajax, templates) {
+        'core/templates',
+        'core/pending'
+        ],
+       function($, notification, Dialogue, str, ajax, templates, Pending) {
 
     /**
      * Constructor
@@ -48,6 +50,7 @@ define(['jquery',
      * @method configureSettings
      */
     settingsMod.prototype.configureSettings = function(e) {
+        var pendingPromise = new Pending();
         var courseid = $(e.target).closest('a').data('courseid');
         var currentValue = $(e.target).closest('a').data('pushratingstouserplans');
         var context = {
@@ -56,16 +59,21 @@ define(['jquery',
         };
         e.preventDefault();
 
-        templates.render('tool_lp/course_competency_settings', context).done(function(html) {
-            str.get_string('configurecoursecompetencysettings', 'tool_lp').done(function(title) {
-                this._dialogue = new Dialogue(
-                    title,
-                    html,
-                    this.addListeners.bind(this)
-                );
-            }.bind(this)).fail(notification.exception);
-        }.bind(this)).fail(notification.exception);
+        $.when(
+            str.get_string('configurecoursecompetencysettings', 'tool_lp'),
+            templates.render('tool_lp/course_competency_settings', context)
+        )
+        .then(function(title, templateResult) {
+            this._dialogue = new Dialogue(
+                title,
+                templateResult[0],
+                this.addListeners.bind(this)
+            );
 
+            return this._dialogue;
+        }.bind(this))
+        .then(pendingPromise.resolve)
+        .catch(notification.exception);
     };
 
     /**
@@ -108,6 +116,7 @@ define(['jquery',
      * @method saveSettings
      */
     settingsMod.prototype.saveSettings = function(e) {
+        var pendingPromise = new Pending();
         e.preventDefault();
 
         var newValue = this._find('input[name="pushratingstouserplans"]:checked').val();
@@ -117,9 +126,12 @@ define(['jquery',
         ajax.call([
             {methodname: 'core_competency_update_course_competency_settings',
               args: {courseid: courseId, settings: settings}}
-        ])[0].done(function() {
-            this.refreshCourseCompetenciesPage();
-        }.bind(this)).fail(notification.exception);
+        ])[0]
+        .then(function() {
+            return this.refreshCourseCompetenciesPage();
+        }.bind(this))
+        .then(pendingPromise.resolve)
+        .catch(notification.exception);
 
     };
 
@@ -131,18 +143,23 @@ define(['jquery',
      */
     settingsMod.prototype.refreshCourseCompetenciesPage = function() {
         var courseId = this._find('input[name="courseid"]').val();
+        var pendingPromise = new Pending();
 
         ajax.call([
             {methodname: 'tool_lp_data_for_course_competencies_page',
               args: {courseid: courseId, moduleid: 0}}
-        ])[0].done(function(context) {
-            templates.render('tool_lp/course_competencies_page', context).done(function(html, js) {
-                $('[data-region="coursecompetenciespage"]').replaceWith(html);
-                templates.runTemplateJS(js);
-                this._dialogue.close();
-            }.bind(this)).fail(notification.exception);
-        }.bind(this)).fail(notification.exception);
+        ])[0]
+        .then(function(context) {
+            return templates.render('tool_lp/course_competencies_page', context);
+        })
+        .then(function(html, js) {
+            templates.replaceNode($('[data-region="coursecompetenciespage"]'), html, js);
+            this._dialogue.close();
 
+            return;
+        }.bind(this))
+        .then(pendingPromise.resolve)
+        .catch(notification.exception);
     };
 
     return /** @alias module:tool_lp/configurecoursecompetencysettings */ settingsMod;

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

@@ -102,15 +102,20 @@ define(['jquery',
      * @return {Promise}
      */
     Grade.prototype.display = function() {
-        return this._render().then(function(html) {
-            return Str.get_string('rate', 'tool_lp').then(function(title) {
-                this._popup = new Dialogue(
-                    title,
-                    html,
-                    this._afterRender.bind(this)
-                );
-            }.bind(this));
-        }.bind(this)).fail(Notification.exception);
+        return $.when(
+            Str.get_string('rate', 'tool_lp'),
+            this._render()
+        )
+        .then(function(title, templateResult) {
+            this._popup = new Dialogue(
+                title,
+                templateResult[0],
+                this._afterRender.bind(this)
+            );
+
+            return this._popup;
+        }.bind(this))
+        .catch(Notification.exception);
     };
 
     /**

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

@@ -95,7 +95,7 @@ define(['jquery',
             self = this;
 
         var promise = ScaleValues.get_values(self._scaleId);
-        promise.done(function(scalevalues) {
+        promise.then(function(scalevalues) {
             options.push({
                 value: '',
                 name: self._chooseStr
@@ -109,8 +109,13 @@ define(['jquery',
                 });
             }
 
-            self._dialogue = new GradeDialogue(options);
-            self._dialogue.on('rated', function(e, data) {
+            return options;
+        })
+        .then(function(options) {
+            return new GradeDialogue(options);
+        })
+        .then(function(dialogue) {
+            dialogue.on('rated', function(e, data) {
                 var args = self._args;
                 args.grade = data.rating;
                 args.note = data.note;
@@ -123,7 +128,15 @@ define(['jquery',
                     fail: notification.exception
                 }]);
             });
-        }).fail(notification.exception);
+
+            return dialogue;
+        })
+        .then(function(dialogue) {
+            self._dialogue = dialogue;
+
+            return;
+        })
+        .fail(notification.exception);
     };
 
     /** @type {Number} The scale id for this competency. */

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,8 +28,10 @@
 
     // 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}}"/>
 {{#hasmodules}}
 <span>
 <label for="module-nav-{{uniqid}}" class="accesshide">{{#str}}filterbyactivity, tool_lp{{/str}}</label>

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,9 +35,12 @@
 
     // 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}}"/>
+<input type="hidden" name="competencyid" value="{{competencyid}}"/>
+<input type="hidden" name="courseid" value="{{courseid}}"/>
 {{#hasusers}}
 <span>
 <label for="user-nav-{{uniqid}}" class="accesshide">{{#str}}jumptouser, tool_lp{{/str}}</label>

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/lp/tests/behat/framework_crud.feature

@@ -6,6 +6,7 @@ Feature: Manage competency frameworks
 
   Background:
     Given I log in as "admin"
+    And I change window size to "small"
     And I am on site homepage
 
   Scenario: Create a new framework

admin/tool/lp/tests/behat/template_crud.feature

@@ -6,6 +6,7 @@ Feature: Manage plearning plan templates
 
   Background:
     Given I log in as "admin"
+    And I change window size to "small"
     And I am on site homepage
 
   Scenario: Create a new learning plan template

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

@@ -105,7 +105,7 @@ $string['requirevalidation'] = 'Validate sender address';
 $string['name'] = 'Name';
 $string['ssl'] = 'SSL (Auto-detect SSL version)';
 $string['sslv2'] = 'SSLv2 (Force SSL Version 2)';
-$string['sslv3'] = 'SSLv2 (Force SSL Version 3)';
+$string['sslv3'] = 'SSLv3 (Force SSL Version 3)';
 $string['taskcleanup'] = 'Cleanup of unverified incoming email';
 $string['taskpickup'] = 'Incoming email pickup';
 $string['tls'] = 'TLS (TLS; started via protocol-level negotiation over unencrypted channel; RECOMMENDED way of initiating secure connection)';

admin/tool/mobile/classes/api.php

@@ -206,6 +206,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.

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

@@ -82,7 +82,7 @@ $string['mobileappearance'] = 'Mobile appearance';
 $string['mobileauthentication'] = 'Mobile authentication';
 $string['mobilecssurl'] = 'CSS';
 $string['mobilefeatures'] = 'Mobile features';
-$string['mobilenotificationsdisabledwarning'] = 'Mobile notifications are not enabled. They should be enabled in Manage message outputs.';
+$string['mobilenotificationsdisabledwarning'] = 'Mobile notifications are not enabled. They should be enabled in Notification settings.';
 $string['mobilesettings'] = 'Mobile settings';
 $string['offlineuse'] = 'Offline use';
 $string['pluginname'] = 'Moodle app tools';
@@ -91,7 +91,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,7 +30,7 @@ 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_NOTAGS); // The URL scheme the app supports.
+$urlscheme         = optional_param('urlscheme', 'moodlemobile', PARAM_ALPHANUM); // 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).
 

admin/tool/mobile/settings.php

@@ -63,7 +63,7 @@ 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_NOTAGS));
+                    new lang_string('forcedurlscheme', 'tool_mobile'), '', PARAM_ALPHANUM));
 
         $ADMIN->add('mobileapp', $temp);
 

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/policy/amd/build/policyactions.min.js

@@ -1 +1 @@
-define(["jquery","core/ajax","core/notification","core/modal_factory","core/modal_events"],function(a,b,c,d,e){var f={VIEW_POLICY:'[data-action="view"]'},g=function(){this.registerEvents()};return g.prototype.registerEvents=function(){a(f.VIEW_POLICY).click(function(f){f.preventDefault();var g=a(this).data("versionid"),h=a(this).data("behalfid"),i={versionid:g,behalfid:h},j={methodname:"tool_policy_get_policy_version",args:i},k=a.Deferred(),l=a.Deferred(),m=d.create({title:k,body:l,large:!0}).then(function(a){return a.getRoot().on(e.hidden,function(){a.destroy()}),a}).then(function(a){return a.show(),a})["catch"](c.exception),n=b.call([j]);a.when(n[0]).then(function(a){if(a.result.policy)return k.resolve(a.result.policy.name),l.resolve(a.result.policy.content),a;throw new Error(a.warnings[0].message)})["catch"](function(a){return m.then(function(a){return a.hide(),a.destroy(),a})["catch"](c.exception),c.addNotification({message:a,type:"error"})})})},{init:function(){return new g}}});
+define(["jquery","core/ajax","core/notification","core/modal_factory","core/modal_events"],function(a,b,c,d,e){var f=function(a){this.registerEvents(a)};return f.prototype.registerEvents=function(f){f.on("click",function(f){f.preventDefault();var g=a(this).data("versionid"),h=a(this).data("behalfid"),i={versionid:g,behalfid:h},j={methodname:"tool_policy_get_policy_version",args:i},k=a.Deferred(),l=a.Deferred(),m=d.create({title:k,body:l,large:!0}).then(function(a){return a.getRoot().on(e.hidden,function(){a.destroy()}),a}).then(function(a){return a.show(),a})["catch"](c.exception),n=b.call([j]);a.when(n[0]).then(function(a){if(a.result.policy)return k.resolve(a.result.policy.name),l.resolve(a.result.policy.content),a;throw new Error(a.warnings[0].message)})["catch"](function(a){return m.then(function(a){return a.hide(),a.destroy(),a})["catch"](c.exception),c.addNotification({message:a,type:"error"})})})},{init:function(b){return b=a(b),new f(b)}}});

admin/tool/policy/amd/src/policyactions.js

@@ -29,27 +29,18 @@ define([
     'core/modal_events'],
 function($, Ajax, Notification, ModalFactory, ModalEvents) {
 
-    /**
-     * List of action selectors.
-     *
-     * @type {{VIEW_POLICY: string}}
-     */
-    var ACTIONS = {
-        VIEW_POLICY: '[data-action="view"]'
-    };
-
     /**
      * PolicyActions class.
      */
-    var PolicyActions = function() {
-        this.registerEvents();
+    var PolicyActions = function(root) {
+        this.registerEvents(root);
     };
 
     /**
      * Register event listeners.
      */
-    PolicyActions.prototype.registerEvents = function() {
-        $(ACTIONS.VIEW_POLICY).click(function(e) {
+    PolicyActions.prototype.registerEvents = function(root) {
+        root.on("click", function(e) {
             e.preventDefault();
 
             var versionid = $(this).data('versionid');
@@ -127,8 +118,9 @@ function($, Ajax, Notification, ModalFactory, ModalEvents) {
          * @method init
          * @return {PolicyActions}
          */
-        'init': function() {
-            return new PolicyActions();
+        'init': function(root) {
+            root = $(root);
+            return new PolicyActions(root);
         }
     };
 });

admin/tool/policy/classes/form/accept_policy.php

@@ -111,7 +111,7 @@ class accept_policy extends \moodleform {
             }
         }
 
-        $PAGE->requires->js_call_amd('tool_policy/policyactions', 'init');
+        $PAGE->requires->js_call_amd('tool_policy/policyactions', 'init', ['[data-action="view"]']);
     }
 
     /**
@@ -197,4 +197,4 @@ class accept_policy extends \moodleform {
             }
         }
     }
-}
+}

admin/tool/policy/templates/guestconsent.mustache

@@ -58,7 +58,7 @@ require(['jquery', 'tool_policy/jquery-eu-cookie-law-popup', 'tool_policy/policy
                    "<ul>{{#policies}}" +
                    "<li>" +
                    "<a href=\"{{pluginbaseurl}}/view.php?versionid={{id}}{{#returnurl}}&amp;returnurl={{.}}{{/returnurl}}\" " +
-                   "   data-action=\"view\" data-versionid=\"{{id}}\" data-behalfid=\"1\" >" +
+                   "   data-action=\"view-guest\" data-versionid=\"{{id}}\" data-behalfid=\"1\" >" +
                    "{{{name}}}" +
                    "</a>" +
                    "</li>" +
@@ -81,7 +81,7 @@ require(['jquery', 'tool_policy/jquery-eu-cookie-law-popup', 'tool_policy/policy
             {{/haspolicies}}
 
             // Initialise the JS for the modal window which displays the policy versions.
-            ActionsMod.init();
+            ActionsMod.init('[data-action="view-guest"]');
         });
 });
 

admin/tool/policy/templates/page_agreedocs.mustache

@@ -143,6 +143,6 @@
 {{#js}}
 // Initialise the JS for the modal window which displays the policy versions.
 require(['tool_policy/policyactions'], function(ActionsMod) {
-    ActionsMod.init();
+    ActionsMod.init('[data-action="view"]');
 });
 {{/js}}

admin/tool/policy/templates/page_nopermission.mustache

@@ -73,6 +73,6 @@
 {{#js}}
 // Initialise the JS for the modal window which displays the policy versions.
 require(['tool_policy/policyactions'], function(ActionsMod) {
-    ActionsMod.init();
+    ActionsMod.init('[data-action="view"]');
 });
 {{/js}}

admin/tool/uploadcourse/classes/course.php

@@ -720,6 +720,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,6 +90,7 @@ $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['missingmandatoryfields'] = 'Missing value for mandatory fields: {$a}';

admin/tool/uploadcourse/tests/course_test.php

@@ -82,6 +82,16 @@ class tool_uploadcourse_course_testcase extends advanced_testcase {
         $this->assertArrayHasKey('invalidshortname', $co->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',
                     ],

admin/tool/usertours/classes/privacy/provider.php

@@ -77,18 +77,22 @@ class provider implements
             }
 
             if ($descriptionidentifier !== null) {
-                $time = transform::datetime($value);
-                $tour = \tool_usertours\tour::instance($tourid);
+                try {
+                    $tour = \tool_usertours\tour::instance($tourid);
+                    $time = transform::datetime($value);
 
-                writer::export_user_preference(
-                    'tool_usertours',
-                    $name,
-                    $time,
-                    get_string($descriptionidentifier, 'tool_usertours', (object) [
-                        'name' => $tour->get_name(),
-                        'time' => $time,
-                    ])
-                );
+                    writer::export_user_preference(
+                        'tool_usertours',
+                        $name,
+                        $time,
+                        get_string($descriptionidentifier, 'tool_usertours', (object) [
+                            'name' => $tour->get_name(),
+                            'time' => $time,
+                        ])
+                    );
+                } catch (\dml_missing_record_exception $ex) {
+                    // The tour related to this user preference no longer exists.
+                }
             }
         }
     }

admin/tool/usertours/tests/privacy_provider_test.php

@@ -112,4 +112,37 @@ class tool_usertours_privacy_testcase extends \core_privacy\tests\provider_testc
 
         $this->assertCount(2, (array) $prefs);
     }
+
+    /**
+     * Ensure that export_user_preferences excludes deleted tours.
+     */
+    public function test_export_user_preferences_deleted_tour() {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $user = \core_user::get_user_by_username('admin');
+
+        $alltours = $DB->get_records('tool_usertours_tours');
+
+        $tour1 = tour::instance(array_shift($alltours)->id);
+        $tour1->mark_user_completed();
+
+        $tour2 = tour::instance(array_shift($alltours)->id);
+        $tour2->mark_user_completed();
+        $tour2->remove();
+
+        $writer = writer::with_context(\context_system::instance());
+
+        provider::export_user_preferences($user->id);
+        $this->assertTrue($writer->has_any_data());
+
+        // We should have one preference.
+        $prefs = $writer->get_user_preferences('tool_usertours');
+        $this->assertCount(1, (array) $prefs);
+
+        // The preference should be related to the first tour.
+        $this->assertContains($tour1->get_name(), reset($prefs)->description);
+    }
 }

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;

admin/user/user_bulk_cohortadd.php

@@ -138,7 +138,7 @@ foreach ($users as $user) {
             '<a href="' . $CFG->wwwroot . '/user/view.php?id=' . $user->id . '&amp;course=' . SITEID . '">' .
             $user->fullname .
             '</a>',
-            $user->email,
+            s($user->email),
             $user->city,
             $user->country,
             $user->lastaccess ? format_time(time() - $user->lastaccess) : $strnever

admin/user/user_bulk_display.php

@@ -72,7 +72,7 @@ foreach($users as $user) {
     $table->data[] = array (
         '<a href="'.$CFG->wwwroot.'/user/view.php?id='.$user->id.'&amp;course='.SITEID.'">'.$user->fullname.'</a>',
 //        $user->username,
-        $user->email,
+        s($user->email),
         $user->city,
         $user->country,
         $user->lastaccess ? format_time(time() - $user->lastaccess) : $strnever

analytics/classes/analysis.php

@@ -138,6 +138,9 @@ class analysis {
                 }
             }
         }
+
+        // Force GC to clean up the indicator instances used during the last iteration.
+        $this->analyser->instantiate_indicators();
     }
 
     /**
@@ -474,6 +477,11 @@ class analysis {
                 list($samplesfeatures, $newindicatorcalculations, $indicatornotnulls) = $rangeindicator->calculate($sampleids,
                     $this->analyser->get_samples_origin(), $range['start'], $range['end'], $prevcalculations);
 
+                // Free memory ASAP.
+                unset($rangeindicator);
+                gc_collect_cycles();
+                gc_mem_caches();
+
                 // Copy the features data to the dataset.
                 foreach ($samplesfeatures as $analysersampleid => $features) {
 
@@ -503,7 +511,7 @@ class analysis {
                         $indcalc->endtime = $range['end'];
                         $indcalc->sampleid = $sampleid;
                         $indcalc->sampleorigin = $this->analyser->get_samples_origin();
-                        $indcalc->indicator = $rangeindicator->get_id();
+                        $indcalc->indicator = $indicator->get_id();
                         $indcalc->value = $calculatedvalue;
                         $indcalc->timecreated = $timecreated;
                         $newcalculations[] = $indcalc;

analytics/classes/insights_generator.php

@@ -135,7 +135,7 @@ class insights_generator {
         $message->component = 'moodle';
         $message->name = 'insights';
 
-        $message->userfrom = \core_user::get_noreply_user();
+        $message->userfrom = \core_user::get_support_user();
         $message->userto = $user;
 
         $message->subject = $this->target->get_insight_subject($this->modelid, $context);
@@ -193,18 +193,23 @@ class insights_generator {
         $insighturl = null;
         foreach ($predictionactions as $action) {
             $actionurl = $action->get_url();
+            $opentoblank = false;
             if (!$actionurl->get_param('forwardurl')) {
 
-                $actiondoneurl = new \moodle_url('/report/insights/done.php');
+                $params = ['actionvisiblename' => $action->get_text(), 'target' => '_blank'];
+                $actiondoneurl = new \moodle_url('/report/insights/done.php', $params);
                 // Set the forward url to the 'done' script.
                 $actionurl->param('forwardurl', $actiondoneurl->out(false));
+
+                $opentoblank = true;
             }
 
             if (empty($insighturl)) {
                 // We use the primary action url as insight url so we log that the user followed the provided link.
                 $insighturl = $action->get_url();
             }
-            $actiondata = (object)['url' => $action->get_url()->out(false), 'text' => $action->get_text()];
+            $actiondata = (object)['url' => $action->get_url()->out(false), 'text' => $action->get_text(),
+                'opentoblank' => $opentoblank];
             $fullmessageplaintext .= get_string('insightinfomessageaction', 'analytics', $actiondata) . PHP_EOL;
             $messageactions[] = $actiondata;
         }

analytics/classes/local/analyser/base.php

@@ -246,6 +246,11 @@ abstract class base {
         foreach ($this->indicators as $key => $indicator) {
             $this->indicators[$key] = call_user_func(array($indicator, 'instance'));
         }
+
+        // Free memory ASAP.
+        gc_collect_cycles();
+        gc_mem_caches();
+
         return $this->indicators;
     }
 

analytics/classes/local/analyser/by_course.php

@@ -50,12 +50,11 @@ abstract class by_course extends base {
         if (!empty($this->options['filter'])) {
             $courses = array();
             foreach ($this->options['filter'] as $courseid) {
-                $courses[$courseid] = new \stdClass();
-                $courses[$courseid]->id = $courseid;
+                $courses[$courseid] = intval($courseid);
             }
 
             list($coursesql, $courseparams) = $DB->get_in_or_equal($courses, SQL_PARAMS_NAMED);
-            $sql .= " AND c.id IN $coursesql";
+            $sql .= " AND c.id $coursesql";
             $params = $params + $courseparams;
         }
 

analytics/classes/local/analysis/result_file.php

@@ -78,9 +78,9 @@ class result_file extends result {
         // if this analyser was analysed less that 1 week ago we skip generating a new one. This
         // helps scale the evaluation process as sites with tons of courses may need a lot of time to
         // complete an evaluation.
-        if (!empty($options['evaluation']) && !empty($options['reuseprevanalysed'])) {
+        if (!empty($this->options['evaluation']) && !empty($this->options['reuseprevanalysed'])) {
 
-            $previousanalysis = \core_analytics\dataset_manager::get_evaluation_analysable_file($this->analyser->get_modelid(),
+            $previousanalysis = \core_analytics\dataset_manager::get_evaluation_analysable_file($this->modelid,
                 $analysable->get_id(), $timesplitting->get_id());
             // 1 week is a partly random time interval, no need to worry about DST.
             $boundary = time() - WEEKSECS;

analytics/classes/local/target/base.php

@@ -95,6 +95,15 @@ abstract class base extends \core_analytics\calculable {
         return true;
     }
 
+    /**
+     * Should the insights of this model be linked from reports?
+     *
+     * @return bool
+     */
+    public function link_insights_report(): bool {
+        return true;
+    }
+
     /**
      * Based on facts (processed by machine learning backends) by default.
      *
@@ -137,7 +146,7 @@ abstract class base extends \core_analytics\calculable {
 
         $actions = array();
 
-        if ($includedetailsaction) {
+        if ($this->link_insights_report() && $includedetailsaction) {
 
             $predictionurl = new \moodle_url('/report/insights/prediction.php', array('id' => $predictionid));
             $detailstext = $this->get_view_details_text();
@@ -229,7 +238,12 @@ abstract class base extends \core_analytics\calculable {
      */
     public function get_insights_users(\context $context) {
         if ($context->contextlevel === CONTEXT_USER) {
-            $users = [$context->instanceid => \core_user::get_user($context->instanceid)];
+            if (!has_capability('moodle/analytics:listowninsights', $context, $context->instanceid)) {
+                $users = [];
+            } else {
+                $users = [$context->instanceid => \core_user::get_user($context->instanceid)];
+            }
+
         } else if ($context->contextlevel >= CONTEXT_COURSE) {
             // At course level or below only enrolled users although this is not ideal for
             // teachers assigned at category level.

analytics/classes/manager.php

@@ -480,6 +480,9 @@ class manager {
     /**
      * Returns the models with insights at the provided context.
      *
+     * Note that this method is used for display purposes. It filters out models whose insights
+     * are not linked from the reports page.
+     *
      * @param \context $context
      * @return \core_analytics\model[]
      */
@@ -490,13 +493,52 @@ class manager {
         $models = self::get_all_models(true, true, $context);
         foreach ($models as $key => $model) {
             // Check that it not only have predictions but also generates insights from them.
-            if (!$model->uses_insights()) {
+            if (!$model->uses_insights() || !$model->get_target()->link_insights_report()) {
                 unset($models[$key]);
             }
         }
         return $models;
     }
 
+    /**
+     * Returns the models that generated insights in the provided context. It can also be used to add new models to the context.
+     *
+     * Note that if you use this function with $newmodelid is the caller responsibility to ensure that the
+     * provided model id generated insights for the provided context.
+     *
+     * @throws \coding_exception
+     * @param  \context $context
+     * @param  int|null $newmodelid A new model to add to the list of models with insights in the provided context.
+     * @return int[]
+     */
+    public static function cached_models_with_insights(\context $context, int $newmodelid = null) {
+
+        $cache = \cache::make('core', 'contextwithinsights');
+        $modelids = $cache->get($context->id);
+        if ($modelids === false) {
+            // The cache is empty, but we don't know if it is empty because there are no insights
+            // in this context or because cache/s have been purged, we need to be conservative and
+            // "pay" 1 db read to fill up the cache.
+
+            $models = \core_analytics\manager::get_models_with_insights($context);
+
+            if ($newmodelid && empty($models[$newmodelid])) {
+                throw new \coding_exception('The provided modelid ' . $newmodelid . ' did not generate any insights');
+            }
+
+            $modelids = array_keys($models);
+            $cache->set($context->id, $modelids);
+
+        } else if ($newmodelid && !in_array($newmodelid, $modelids)) {
+            // We add the context we got as an argument to the cache.
+
+            array_push($modelids, $newmodelid);
+            $cache->set($context->id, $modelids);
+        }
+
+        return $modelids;
+    }
+
     /**
      * Returns a prediction
      *
@@ -549,30 +591,32 @@ class manager {
     public static function cleanup() {
         global $DB;
 
-        // Clean up stuff that depends on contexts that do not exist anymore.
-        $sql = "SELECT DISTINCT ap.contextid FROM {analytics_predictions} ap
-                  LEFT JOIN {context} ctx ON ap.contextid = ctx.id
-                 WHERE ctx.id IS NULL";
-        $apcontexts = $DB->get_records_sql($sql);
+        $DB->execute("DELETE FROM {analytics_prediction_actions} WHERE predictionid IN
+                          (SELECT ap.id FROM {analytics_predictions} ap
+                        LEFT JOIN {context} ctx ON ap.contextid = ctx.id
+                            WHERE ctx.id IS NULL)");
 
-        $sql = "SELECT DISTINCT aic.contextid FROM {analytics_indicator_calc} aic
-                  LEFT JOIN {context} ctx ON aic.contextid = ctx.id
-                 WHERE ctx.id IS NULL";
-        $indcalccontexts = $DB->get_records_sql($sql);
-
-        $contexts = $apcontexts + $indcalccontexts;
-        if ($contexts) {
-            list($sql, $params) = $DB->get_in_or_equal(array_keys($contexts));
-            $DB->execute("DELETE FROM {analytics_prediction_actions} WHERE predictionid IN
-                (SELECT ap.id FROM {analytics_predictions} ap WHERE ap.contextid $sql)", $params);
-
-            $DB->delete_records_select('analytics_predictions', "contextid $sql", $params);
-            $DB->delete_records_select('analytics_indicator_calc', "contextid $sql", $params);
-        }
+        $contextsql = "SELECT id FROM {context} ctx";
+        $DB->delete_records_select('analytics_predictions', "contextid NOT IN ($contextsql)");
+        $DB->delete_records_select('analytics_indicator_calc', "contextid NOT IN ($contextsql)");
 
         // Clean up stuff that depends on analysable ids that do not exist anymore.
+
         $models = self::get_all_models();
         foreach ($models as $model) {
+
+            // We first dump into memory the list of analysables we have in the database (we could probably do this with 1 single
+            // query for the 3 tables, but it may be safer to do it separately).
+            $predictsamplesanalysableids = $DB->get_fieldset_select('analytics_predict_samples', 'DISTINCT analysableid',
+                'modelid = :modelid', ['modelid' => $model->get_id()]);
+            $predictsamplesanalysableids = array_flip($predictsamplesanalysableids);
+            $trainsamplesanalysableids = $DB->get_fieldset_select('analytics_train_samples', 'DISTINCT analysableid',
+                'modelid = :modelid', ['modelid' => $model->get_id()]);
+            $trainsamplesanalysableids = array_flip($trainsamplesanalysableids);
+            $usedanalysablesanalysableids = $DB->get_fieldset_select('analytics_used_analysables', 'DISTINCT analysableid',
+                'modelid = :modelid', ['modelid' => $model->get_id()]);
+            $usedanalysablesanalysableids = array_flip($usedanalysablesanalysableids);
+
             $analyser = $model->get_analyser(array('notimesplitting' => true));
             $analysables = $analyser->get_analysables_iterator();
 
@@ -581,17 +625,28 @@ class manager {
                 if (!$analysable) {
                     continue;
                 }
-                $analysableids[] = $analysable->get_id();
-            }
-            if (empty($analysableids)) {
-                continue;
+                unset($predictsamplesanalysableids[$analysable->get_id()]);
+                unset($trainsamplesanalysableids[$analysable->get_id()]);
+                unset($usedanalysablesanalysableids[$analysable->get_id()]);
             }
 
-            list($notinsql, $params) = $DB->get_in_or_equal($analysableids, SQL_PARAMS_NAMED, 'param', false);
-            $params['modelid'] = $model->get_id();
+            $param = ['modelid' => $model->get_id()];
 
-            $DB->delete_records_select('analytics_predict_samples', "modelid = :modelid AND analysableid $notinsql", $params);
-            $DB->delete_records_select('analytics_train_samples', "modelid = :modelid AND analysableid $notinsql", $params);
+            if ($predictsamplesanalysableids) {
+                list($idssql, $idsparams) = $DB->get_in_or_equal(array_flip($predictsamplesanalysableids), SQL_PARAMS_NAMED);
+                $DB->delete_records_select('analytics_predict_samples', "modelid = :modelid AND analysableid $idssql",
+                    $param + $idsparams);
+            }
+            if ($trainsamplesanalysableids) {
+                list($idssql, $idsparams) = $DB->get_in_or_equal(array_flip($trainsamplesanalysableids), SQL_PARAMS_NAMED);
+                $DB->delete_records_select('analytics_train_samples', "modelid = :modelid AND analysableid $idssql",
+                    $param + $idsparams);
+            }
+            if ($usedanalysablesanalysableids) {
+                list($idssql, $idsparams) = $DB->get_in_or_equal(array_flip($usedanalysablesanalysableids), SQL_PARAMS_NAMED);
+                $DB->delete_records_select('analytics_used_analysables', "modelid = :modelid AND analysableid $idssql",
+                    $param + $idsparams);
+            }
         }
     }
 

analytics/classes/model.php

@@ -933,19 +933,11 @@ class model {
 
         $this->get_target()->generate_insight_notifications($this->model->id, $samplecontexts, $predictions);
 
-        // Update cache.
-        $cache = \cache::make('core', 'contextwithinsights');
-        foreach ($samplecontexts as $context) {
-            $modelids = $cache->get($context->id);
-            if (!$modelids) {
-                // The cache is empty, but we don't know if it is empty because there are no insights
-                // in this context or because cache/s have been purged, we need to be conservative and
-                // "pay" 1 db read to fill up the cache.
-                $models = \core_analytics\manager::get_models_with_insights($context);
-                $cache->set($context->id, array_keys($models));
-            } else if (!in_array($this->get_id(), $modelids)) {
-                array_push($modelids, $this->get_id());
-                $cache->set($context->id, $modelids);
+        if ($this->get_target()->link_insights_report()) {
+
+            // Update cache.
+            foreach ($samplecontexts as $context) {
+                \core_analytics\manager::cached_models_with_insights($context, $this->get_id());
             }
         }
     }
@@ -1000,7 +992,7 @@ class model {
         }
 
         // Get all samples data.
-        list($sampleids, $samplesdata) = $this->get_analyser()->get_samples($sampleids);
+        list($sampleids, $samplesdata) = $this->get_samples($sampleids);
 
         // Calculate the targets.
         $predictions = array();
@@ -1314,7 +1306,7 @@ class model {
             return $prediction->sampleid;
         }, $predictions);
 
-        list($unused, $samplesdata) = $this->get_analyser()->get_samples($sampleids);
+        list($unused, $samplesdata) = $this->get_samples($sampleids);
 
         $current = 0;
 
@@ -1356,7 +1348,7 @@ class model {
      */
     public function prediction_sample_data($predictionobj) {
 
-        list($unused, $samplesdata) = $this->get_analyser()->get_samples(array($predictionobj->sampleid));
+        list($unused, $samplesdata) = $this->get_samples(array($predictionobj->sampleid));
 
         if (empty($samplesdata[$predictionobj->sampleid])) {
             throw new \moodle_exception('errorsamplenotavailable', 'analytics');
@@ -1668,12 +1660,8 @@ class model {
             $predictor->clear_model($this->get_unique_id(), $this->get_output_dir());
         }
 
-        $predictionids = $DB->get_fieldset_select('analytics_predictions', 'id', 'modelid = :modelid',
-            array('modelid' => $this->get_id()));
-        if ($predictionids) {
-            list($sql, $params) = $DB->get_in_or_equal($predictionids);
-            $DB->delete_records_select('analytics_prediction_actions', "predictionid $sql", $params);
-        }
+        $DB->delete_records_select('analytics_prediction_actions', "predictionid IN
+            (SELECT id FROM {analytics_predictions} WHERE modelid = :modelid)", ['modelid' => $this->get_id()]);
 
         $DB->delete_records('analytics_predictions', array('modelid' => $this->model->id));
         $DB->delete_records('analytics_predict_samples', array('modelid' => $this->model->id));
@@ -1759,30 +1747,94 @@ class model {
         $contextids = array_map(function($predictionobj) {
             return $predictionobj->contextid;
         }, $predictionrecords);
-        list($contextsql, $contextparams) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED);
 
-        // We select the fields that will allow us to map ids to $predictionrecords. Given that we already filter by modelid
-        // we have enough with sampleid and rangeindex. The reason is that the sampleid relation to a site is N - 1.
-        $fields = 'id, sampleid, rangeindex';
+        // Limited to 30000 records as a middle point between the ~65000 params limit in pgsql and the size limit for mysql which
+        // can be increased if required up to a reasonable point.
+        $chunks = array_chunk($contextids, 30000);
+        foreach ($chunks as $contextidschunk) {
+            list($contextsql, $contextparams) = $DB->get_in_or_equal($contextidschunk, SQL_PARAMS_NAMED);
 
-        // We include the contextid and the timecreated filter to reduce the number of records in $dbpredictions. We can not
-        // add as many OR conditions as records in $predictionrecords.
-        $sql = "SELECT $fields
-                  FROM {analytics_predictions}
-                 WHERE modelid = :modelid
-                       AND contextid $contextsql
-                       AND timecreated >= :firsttimecreated";
-        $params = $contextparams + ['modelid' => $this->model->id, 'firsttimecreated' => $firstprediction->timecreated];
-        $dbpredictions = $DB->get_recordset_sql($sql, $params);
-        foreach ($dbpredictions as $id => $dbprediction) {
-            // The append_rangeindex implementation is the same regardless of the time splitting method in use.
-            $uniqueid = $this->get_time_splitting()->append_rangeindex($dbprediction->sampleid, $dbprediction->rangeindex);
-            $predictionrecords[$uniqueid]->id = $dbprediction->id;
+            // We select the fields that will allow us to map ids to $predictionrecords. Given that we already filter by modelid
+            // we have enough with sampleid and rangeindex. The reason is that the sampleid relation to a site is N - 1.
+            $fields = 'id, sampleid, rangeindex';
+
+            // We include the contextid and the timecreated filter to reduce the number of records in $dbpredictions. We can not
+            // add as many OR conditions as records in $predictionrecords.
+            $sql = "SELECT $fields
+                      FROM {analytics_predictions}
+                     WHERE modelid = :modelid
+                           AND contextid $contextsql
+                           AND timecreated >= :firsttimecreated";
+            $params = $contextparams + ['modelid' => $this->model->id, 'firsttimecreated' => $firstprediction->timecreated];
+            $dbpredictions = $DB->get_recordset_sql($sql, $params);
+            foreach ($dbpredictions as $id => $dbprediction) {
+                // The append_rangeindex implementation is the same regardless of the time splitting method in use.
+                $uniqueid = $this->get_time_splitting()->append_rangeindex($dbprediction->sampleid, $dbprediction->rangeindex);
+                $predictionrecords[$uniqueid]->id = $dbprediction->id;
+            }
         }
 
         return $predictionrecords;
     }
 
+    /**
+     * Wrapper around analyser's get_samples to skip DB's max-number-of-params exception.
+     *
+     * @param  array  $sampleids
+     * @return array
+     */
+    public function get_samples(array $sampleids): array {
+
+        if (empty($sampleids)) {
+            throw new \coding_exception('No sample ids provided');
+        }
+
+        $chunksize = count($sampleids);
+
+        // We start with just 1 chunk, if it is too large for the db we split the list of sampleids in 2 and we
+        // try again. We repeat this process until the chunk is small enough for the db engine to process. The
+        // >= has been added in case there are other \dml_read_exceptions unrelated to the max number of params.
+        while (empty($done) && $chunksize >= 1) {
+
+            $chunks = array_chunk($sampleids, $chunksize);
+            $allsampleids = [];
+            $allsamplesdata = [];
+
+            foreach ($chunks as $index => $chunk) {
+
+                try {
+                    list($chunksampleids, $chunksamplesdata) = $this->get_analyser()->get_samples($chunk);
+                } catch (\dml_read_exception $e) {
+
+                    // Reduce the chunksize, we use floor() so the $chunksize is always less than the previous $chunksize value.
+                    $chunksize = floor($chunksize / 2);
+                    break;
+                }
+
+                // We can sum as these two arrays are indexed by sampleid and there are no collisions.
+                $allsampleids = $allsampleids + $chunksampleids;
+                $allsamplesdata = $allsamplesdata + $chunksamplesdata;
+
+                if ($index === count($chunks) - 1) {
+                    // We successfully processed all the samples in all chunks, we are done.
+                    $done = true;
+                }
+            }
+        }
+
+        if (empty($done)) {
+            if (!empty($e)) {
+                // Throw the last exception we caught, the \dml_read_exception we have been catching is unrelated to the max number
+                // of param's exception.
+                throw new \dml_read_exception($e);
+            } else {
+                throw new \coding_exception('We should never reach this point, there is a bug in ' .
+                    'core_analytics\\model::get_samples\'s code');
+            }
+        }
+        return [$allsampleids, $allsamplesdata];
+    }
+
     /**
      * Purges the insights cache.
      */

analytics/classes/privacy/provider.php

@@ -306,13 +306,9 @@ class provider implements
             $idssql = "SELECT ap.id FROM {analytics_predictions} ap
                         WHERE ap.contextid = :contextid AND ap.modelid = :modelid";
             $idsparams = ['contextid' => $context->id, 'modelid' => $modelid];
-            $predictionids = $DB->get_fieldset_sql($idssql, $idsparams);
-            if ($predictionids) {
-                list($predictionidssql, $params) = $DB->get_in_or_equal($predictionids, SQL_PARAMS_NAMED);
 
-                $DB->delete_records_select('analytics_prediction_actions', "predictionid IN ($idssql)", $idsparams);
-                $DB->delete_records_select('analytics_predictions', "id $predictionidssql", $params);
-            }
+            $DB->delete_records_select('analytics_prediction_actions', "predictionid IN ($idssql)", $idsparams);
+            $DB->delete_records_select('analytics_predictions', "contextid = :contextid AND modelid = :modelid", $idsparams);
         }
 
         // We delete them all this table is just a cache and we don't know which model filled it.

analytics/templates/insight_info_message_prediction.mustache

@@ -33,7 +33,8 @@
                 "text": "Moodle"
             }, {
                 "url": "https://en.wikipedia.org/wiki/Noodle",
-                "text": "Noodle"
+                "text": "Noodle",
+                "opentoblank": 1
             }
         ]
     }
@@ -65,5 +66,5 @@ body:not(.dir-ltr):not(.dir-rtl) .btn-insight {
 
 <br/>
 {{#actions}}
-    <a class="btn btn-default m-r-1 m-b-1 btn-insight" href="{{url}}">{{text}}</a>
+    <a class="btn btn-default m-r-1 m-b-1 btn-insight" {{#opentoblank}}target="_blank" {{/opentoblank}}href="{{url}}">{{text}}</a>
 {{/actions}}