Moodle release 4.4.4

We are encountering Behat failures on MacOS and possibly Windows.

This reverts commit 3202109780.

.github/workflows/onebyone.yml

@@ -0,0 +1,143 @@
+name: One by One Testing
+# Run all the individual unit tests one by one, with
+# fully independent PHPUnit executions. Useful to
+# detect issues with some tests that are using stuff
+# that has been made available by others, but is not
+# available when running individually.
+#
+# Note that we aren't using PHPUnit's own isolation
+# here but completely separated runs, one for each
+# test.
+#
+# The workflow will fail reporting all the tests
+# that have failed (and will pass if no failure is
+# detected, of course).
+#
+# It's only executed via workflow dispatch (automated
+# or manual), not by push/tag. And acceptd configuration
+# of phpunit, specially useful to run it with PHPUnit's
+# own isolation or any other option.
+
+on:
+  workflow_dispatch:
+    inputs:
+      phpunit_extra_options:
+        description: Additional options to apply to PHPUnit
+        required: false
+        default: ''
+
+env:
+  chunks: 7
+
+jobs:
+  collect:
+    name: Collect individual unit tests
+    runs-on: ubuntu-latest
+    outputs:
+      matrix: ${{steps.individual-tests.outputs.matrix }}
+
+    steps:
+      - name: Checking out code
+        uses: actions/checkout@v4
+
+      - name: Looking for all individual tests
+        id: individual-tests
+        run: |
+          count=0  # Number of individual tests found.
+          while read -r testfile; do # For each test file.
+              while read -r testname; do # For each unit test in a file.
+                  count=$((count + 1))
+                  # Sent it to the correct chunk file.
+                  chunk=$(((($count % $chunks)) + 1))
+                  echo "$testname $testfile" >> ./chunk_$chunk.txt
+              done < <(grep "function test_" "${testfile}" | sed -r "s/^.*function (test_[a-zA-Z0-9_]+).*/\1/")
+          done < <(find . -name "*_test.php")
+          # Generate the matrix to run tests.
+          echo "matrix=$(ls -1 chunk_*.txt | jq -R -s -c 'split("\n")[:-1]')" >> $GITHUB_OUTPUT
+          echo "$count individual tests collected in $chunks files"
+
+      - name: Upload individual tests files
+        uses: actions/upload-artifact@v4
+        with:
+          name: individual_tests
+          path: chunk_*.txt
+          retention-days: 1
+
+  test:
+    name: Run tests
+    needs: collect
+    runs-on: ubuntu-latest
+    services:
+      exttests:
+        image: moodlehq/moodle-exttests
+        ports:
+          - 8080:80
+      redis:
+        image: redis
+        ports:
+          - 6379:6379
+    strategy:
+      fail-fast: false
+      matrix:
+        file: ${{ fromJson(needs.collect.outputs.matrix) }}
+
+    steps:
+      - name: Setting up DB pgsql
+        uses: m4nu56/postgresql-action@v1
+        with:
+          postgresql version: 13
+          postgresql db: test
+          postgresql user: test
+          postgresql password: test
+
+      - name: Setting up PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 8.3
+          ini-values: max_input_vars=5000
+          coverage: none
+
+      - name: Checking out code
+        uses: actions/checkout@v4
+
+      - name: Download individual test files
+        uses: actions/download-artifact@v4
+        with:
+          name: individual_tests # Make all the chunk files available for the next steps.
+
+      - name: Setting up PHPUnit
+        env:
+          dbtype: pgsql
+        run: |
+          echo "pathtophp=$(which php)" >> $GITHUB_ENV
+          cp .github/workflows/config-template.php config.php
+          mkdir ../moodledata
+          sudo locale-gen en_AU.UTF-8
+          php admin/tool/phpunit/cli/init.php --no-composer-self-update
+
+      - name: Run PHPUnit test (one by one)
+        env:
+          dbtype: pgsql
+        run: |
+          status=0
+          count=0
+          while read -r line; do # For each line in the chunk file
+              count=$((count + 1))
+              filter="${line% *}"
+              file="${line#* }"
+              # Run the individual unit test and report problems if needed to.
+              if ! php vendor/bin/phpunit \
+                      --fail-on-empty-test-suite \
+                      --fail-on-warning \
+                      --fail-on-risky \
+                      --filter "$filter" ${{ inputs.phpunit_extra_options }} \
+                      "$file" >/dev/null 2>&1; then
+                  if [ $status -eq 0 ]; then
+                      echo "Problems found, list of PHPUnit commands failing:"
+                  fi
+                  echo "vendor/bin/phpunit --filter '${filter}' ${{ inputs.phpunit_extra_options }} $file"
+                  status=$((status + 1))
+              fi
+          done < ${{ matrix.file }}
+          echo "Finished: $count individual tests executed, $status tests failed"
+          exit $status

.github/workflows/push.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches-ignore:
       - main
-      - master
       - MOODLE_[0-9]+_STABLE
     tags-ignore:
       - v[0-9]+.[0-9]+.[0-9]+*

.github/workflows/web-installer-test.yml

@@ -0,0 +1,23 @@
+name: Web Installer Testing
+
+on:
+  workflow_dispatch:
+
+jobs:
+  InstallationTest:
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest]
+        php: [8.1, 8.3]
+    steps:
+      - name: Construct repository URL
+        run: echo "REPOSITORY_URL=${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}" >> $GITHUB_ENV
+        shell: bash
+
+      - uses: moodlehq/moodle-webinstaller-test@main
+        with:
+          repository: ${{ env.REPOSITORY_URL }}
+          branch: ${{ github.ref_name }}
+          php: ${{ matrix.php }}

.gitignore

@@ -26,7 +26,9 @@
 /.patches/
 /.idea/
 .phpstorm.*
-!.phpstorm.meta.php
+!/.phpstorm.meta.php/
+/.phpstorm.meta.php/*
+!/.phpstorm.meta.php/di.php
 /nbproject/
 CVS
 .DS_Store

admin/amd/build/bulk_user_actions.min.js

@@ -5,6 +5,6 @@ define("core_admin/bulk_user_actions",["exports","core_reportbuilder/local/selec
    * @module     core_admin/bulk_user_actions
    * @copyright  2024 Marina Glancy
    * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
-   */Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.init=void 0,reportSelectors=_interopRequireWildcard(reportSelectors),tableEvents=_interopRequireWildcard(tableEvents),FormChangeChecker=_interopRequireWildcard(FormChangeChecker),CustomEvents=_interopRequireWildcard(CustomEvents),_jquery=(obj=_jquery)&&obj.__esModule?obj:{default:obj};const Selectors_bulkActionsForm="form#user-bulk-action-form",Selectors_userReportWrapper='[data-region="report-user-list-wrapper"]',Selectors_checkbox='input[type="checkbox"][data-togglegroup="report-select-all"][data-toggle="slave"]',Selectors_masterCheckbox='input[type="checkbox"][data-togglegroup="report-select-all"][data-toggle="master"]',Selectors_checkedRows='[data-togglegroup="report-select-all"][data-toggle="slave"]:checked';_exports.init=()=>{var _userBulkForm$closest;const userBulkForm=document.querySelector(Selectors_bulkActionsForm),userReport=null==userBulkForm||null===(_userBulkForm$closest=userBulkForm.closest(Selectors_userReportWrapper))||void 0===_userBulkForm$closest?void 0:_userBulkForm$closest.querySelector(reportSelectors.regions.report);if(!userBulkForm||!userReport)return;const actionSelect=userBulkForm.querySelector("select");CustomEvents.define(actionSelect,[CustomEvents.events.accessibleChange]),(0,_jquery.default)(actionSelect).on(CustomEvents.events.accessibleChange,(event=>{if(event.target.value&&"0"!=="".concat(event.target.value)){const e=new Event("submit",{cancelable:!0});userBulkForm.dispatchEvent(e),e.defaultPrevented||(FormChangeChecker.markFormSubmitted(userBulkForm),userBulkForm.submit())}}));const updateUserIds=()=>{const selectedUsers=[...userReport.querySelectorAll(Selectors_checkedRows)],selectedUserIds=selectedUsers.map((check=>parseInt(check.value)));userBulkForm.querySelector('[name="userids"]').value=selectedUserIds.join(","),actionSelect.disabled=0===selectedUsers.length;const selectedUsersNames=selectedUsers.map((check=>document.querySelector('label[for="'.concat(check.id,'"]')).textContent));userBulkForm.data={userids:selectedUserIds,usernames:selectedUsersNames}};updateUserIds(),document.addEventListener("change",(event=>{(event.target.matches(Selectors_checkbox)||event.target.matches(Selectors_masterCheckbox))&&userReport.contains(event.target)&&updateUserIds()})),document.addEventListener(tableEvents.tableContentRefreshed,(event=>{userReport.contains(event.target)&&updateUserIds()}))}}));
+   */Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.init=void 0,reportSelectors=_interopRequireWildcard(reportSelectors),tableEvents=_interopRequireWildcard(tableEvents),FormChangeChecker=_interopRequireWildcard(FormChangeChecker),CustomEvents=_interopRequireWildcard(CustomEvents),_jquery=(obj=_jquery)&&obj.__esModule?obj:{default:obj};const Selectors_bulkActionsForm="form#user-bulk-action-form",Selectors_userReportWrapper='[data-region="report-user-list-wrapper"]',Selectors_checkbox='input[type="checkbox"][data-togglegroup="report-select-all"][data-toggle="slave"]',Selectors_masterCheckbox='input[type="checkbox"][data-togglegroup="report-select-all"][data-toggle="master"]',Selectors_checkedRows='[data-togglegroup="report-select-all"][data-toggle="slave"]:checked';_exports.init=()=>{var _userBulkForm$closest;const userBulkForm=document.querySelector(Selectors_bulkActionsForm),userReport=null==userBulkForm||null===(_userBulkForm$closest=userBulkForm.closest(Selectors_userReportWrapper))||void 0===_userBulkForm$closest?void 0:_userBulkForm$closest.querySelector(reportSelectors.regions.report);if(!userBulkForm||!userReport)return;const actionSelect=userBulkForm.querySelector("select");CustomEvents.define(actionSelect,[CustomEvents.events.accessibleChange]),(0,_jquery.default)(actionSelect).on(CustomEvents.events.accessibleChange,(event=>{if(event.target.value&&"0"!=="".concat(event.target.value)){const e=new Event("submit",{cancelable:!0});userBulkForm.dispatchEvent(e),e.defaultPrevented||(FormChangeChecker.markFormSubmitted(userBulkForm),userBulkForm.submit())}}));const updateUserIds=()=>{const selectedUsers=[...userReport.querySelectorAll(Selectors_checkedRows)],selectedUserIds=selectedUsers.map((check=>parseInt(check.value)));userBulkForm.querySelector('[name="userids"]').value=selectedUserIds.join(","),actionSelect.disabled=0===selectedUsers.length,actionSelect.disabled&&(actionSelect.value="0");const selectedUsersNames=selectedUsers.map((check=>document.querySelector('label[for="'.concat(check.id,'"]')).textContent));userBulkForm.data={userids:selectedUserIds,usernames:selectedUsersNames}};updateUserIds(),document.addEventListener("change",(event=>{(event.target.matches(Selectors_checkbox)||event.target.matches(Selectors_masterCheckbox))&&userReport.contains(event.target)&&updateUserIds()})),document.addEventListener(tableEvents.tableContentRefreshed,(event=>{userReport.contains(event.target)&&updateUserIds()}))}}));
 
 //# sourceMappingURL=bulk_user_actions.min.js.map

admin/amd/src/bulk_user_actions.js

@@ -65,7 +65,13 @@ export const init = () => {
         const selectedUsers = [...userReport.querySelectorAll(Selectors.checkedRows)];
         const selectedUserIds = selectedUsers.map(check => parseInt(check.value));
         userBulkForm.querySelector('[name="userids"]').value = selectedUserIds.join(',');
+
+        // Disable the action selector if nothing selected, and reset the current selection.
         actionSelect.disabled = selectedUsers.length === 0;
+        if (actionSelect.disabled) {
+            actionSelect.value = "0";
+        }
+
         const selectedUsersNames = selectedUsers.map(check => document.querySelector(`label[for="${check.id}"]`).textContent);
         // Add the user ids and names to the form data attributes so they can be available from the
         // other JS modules that listen to the form submit event.

admin/classes/reportbuilder/local/systemreports/users.php

@@ -184,6 +184,7 @@ class users extends system_report {
         $entityuseralias = $entityuser->get_table_alias('user');
 
         $filters = [
+            'user:fullname',
             'user:firstname',
             'user:lastname',
             'user:username',

admin/classes/table/plugin_management_table.php

@@ -500,4 +500,15 @@ abstract class plugin_management_table extends flexible_table implements dynamic
     protected function supports_ordering(): bool {
         return $this->plugininfoclass::plugintype_supports_ordering();
     }
+
+    /**
+     * Check if the user has the capability to access this table.
+     *
+     * Default implementation for plugin management tables is to require 'moodle/site:config' capability
+     *
+     * @return bool Return true if capability check passed.
+     */
+    public function has_capability(): bool {
+        return has_capability('moodle/site:config', $this->get_context());
+    }
 }

admin/cli/install.php

@@ -177,6 +177,7 @@ $CFG->admin                = array_pop($parts);
 ini_set('include_path', $CFG->libdir.'/pear' . PATH_SEPARATOR . ini_get('include_path'));
 
 require_once($CFG->libdir.'/classes/component.php');
+
 require_once($CFG->libdir.'/classes/text.php');
 require_once($CFG->libdir.'/classes/string_manager.php');
 require_once($CFG->libdir.'/classes/string_manager_install.php');
@@ -192,13 +193,8 @@ require_once($CFG->libdir.'/adminlib.php');
 require_once($CFG->libdir.'/componentlib.class.php');
 require_once($CFG->dirroot.'/cache/lib.php');
 
-// Register our classloader, in theory somebody might want to replace it to load other hacked core classes.
-// Required because the database checks below lead to session interaction which is going to lead us to requiring autoloaded classes.
-if (defined('COMPONENT_CLASSLOADER')) {
-    spl_autoload_register(COMPONENT_CLASSLOADER);
-} else {
-    spl_autoload_register('core_component::classloader');
-}
+// Register our classloader.
+\core_component::register_autoloader();
 
 require($CFG->dirroot.'/version.php');
 $CFG->target_release = $release;

admin/environment.xml

@@ -4508,4 +4508,199 @@
       </CUSTOM_CHECK>
     </CUSTOM_CHECKS>
   </MOODLE>
+  <MOODLE version="4.5" requires="4.1.2">
+    <UNICODE level="required">
+      <FEEDBACK>
+        <ON_ERROR message="unicoderequired" />
+      </FEEDBACK>
+    </UNICODE>
+    <DATABASE level="required">
+      <VENDOR name="mariadb" version="10.6.7" />
+      <VENDOR name="mysql" version="8.0" />
+      <VENDOR name="postgres" version="13" />
+      <VENDOR name="mssql" version="14.0" />
+      <VENDOR name="oracle" version="19" />
+    </DATABASE>
+    <PHP version="8.1.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="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_EXTENSION name="sodium" level="required"/>
+      <PHP_EXTENSION name="exif" level="optional"/>
+    </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="required">
+        <FEEDBACK>
+          <ON_ERROR message="sixtyfourbitsrequired" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_max_input_vars" level="optional">
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_admin_dir_usage" level="optional">
+        <FEEDBACK>
+          <ON_CHECK message="iscustomadminwarnings" />
+        </FEEDBACK>
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_xmlrpc_usage" level="optional">
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_mod_assignment" level="required">
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_db_prefix_length" level="required">
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_oracle_usage" level="optional">
+      </CUSTOM_CHECK>
+      <CUSTOM_CHECK file="lib/upgradelib.php" function="check_async_backup" level="recommended">
+      </CUSTOM_CHECK>
+    </CUSTOM_CHECKS>
+  </MOODLE>
 </COMPATIBILITY_MATRIX>

admin/presets/tests/manager_test.php

@@ -29,6 +29,14 @@ use stdClass;
  * @coversDefaultClass \core_adminpresets\manager
  */
 class manager_test extends \advanced_testcase {
+    /**
+     * Include required libraries.
+     */
+    public static function setUpBeforeClass(): void {
+        global $CFG;
+        require_once($CFG->libdir.'/adminlib.php');
+    }
+
     /**
      * Test the behaviour of protected get_site_settings method.
      *

admin/presets/tests/privacy/privacy_provider_test.php

@@ -36,7 +36,7 @@ class privacy_provider_test extends provider_testcase {
      * Test for provider::get_metadata().
      * @covers ::get_metadata
      */
-    public function test_get_metadata() {
+    public function test_get_metadata(): void {
         $collection = new collection('core_adminpresets');
         $newcollection = provider::get_metadata($collection);
         $itemcollection = $newcollection->get_collection();
@@ -49,7 +49,7 @@ class privacy_provider_test extends provider_testcase {
      * Test for provider::get_contexts_for_userid() doesn't return any context.
      * @covers ::get_contexts_for_userid
      */
-    public function test_get_contexts_for_userid() {
+    public function test_get_contexts_for_userid(): void {
         global $USER;
 
         $this->resetAfterTest();
@@ -67,7 +67,7 @@ class privacy_provider_test extends provider_testcase {
      * Test for provider::get_users_in_context() doesn't return any user.
      * @covers ::get_users_in_context
      */
-    public function test_get_users_in_context() {
+    public function test_get_users_in_context(): void {
         global $USER;
 
         $this->resetAfterTest();
@@ -88,7 +88,7 @@ class privacy_provider_test extends provider_testcase {
      * Test for provider::export_user_data().
      * @covers ::export_user_data
      */
-    public function test_export_user_data() {
+    public function test_export_user_data(): void {
         global $USER;
 
         $this->resetAfterTest();
@@ -121,7 +121,7 @@ class privacy_provider_test extends provider_testcase {
      * Test for provider::delete_data_for_all_users_in_context().
      * @covers ::delete_data_for_all_users_in_context
      */
-    public function test_delete_data_for_all_users_in_context() {
+    public function test_delete_data_for_all_users_in_context(): void {
         global $DB, $USER;
 
         $this->resetAfterTest();
@@ -146,7 +146,7 @@ class privacy_provider_test extends provider_testcase {
      * Test for provider::delete_data_for_user().
      * @covers ::delete_data_for_user
      */
-    public function test_delete_data_for_user() {
+    public function test_delete_data_for_user(): void {
         global $DB, $USER;
 
         $this->resetAfterTest();

admin/renderer.php

@@ -874,8 +874,8 @@ class core_admin_renderer extends plugin_renderer_base {
     protected function mobile_configuration_warning($mobileconfigured) {
         $output = '';
         if (!$mobileconfigured) {
-            $settingslink = new moodle_url('/admin/settings.php', ['section' => 'mobilesettings']);
-            $configurebutton = $this->single_button($settingslink, get_string('enablemobilewebservice', 'admin'));
+            $settingslink = new moodle_url('/admin/search.php', ['query' => 'enablemobilewebservice']);
+            $configurebutton = $this->single_button($settingslink, get_string('enablemobilewebservice', 'admin'), 'get');
             $output .= $this->warning(get_string('mobilenotconfiguredwarning', 'admin') . ' ' . $configurebutton);
         }
 

admin/roles/tests/preset_test.php

@@ -27,7 +27,7 @@ use core_role_preset;
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class preset_test extends \advanced_testcase {
-    public function test_xml() {
+    public function test_xml(): void {
         global $DB;
 
         $roles = $DB->get_records('role');
@@ -81,7 +81,7 @@ class preset_test extends \advanced_testcase {
      * Tests covered method.
      * @covers \core_role_preset::parse_preset
      */
-    public function test_mixed_levels() {
+    public function test_mixed_levels(): void {
         // The problem here is that we cannot guarantee plugin contexts
         // have unique short names, so we have to also support level numbers.
         $xml = file_get_contents(__DIR__ . '/fixtures/mixed_levels.xml');

admin/roles/tests/privacy/provider_test.php

@@ -45,7 +45,7 @@ class provider_test extends provider_testcase {
      * Test to check export_user_preferences.
      * returns user preferences data.
      */
-    public function test_export_user_preferences() {
+    public function test_export_user_preferences(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
         $user = $this->getDataGenerator()->create_user();
@@ -64,7 +64,7 @@ class provider_test extends provider_testcase {
     /**
      * Check all contexts are returned if there is any user data for this user.
      */
-    public function test_get_contexts_for_userid() {
+    public function test_get_contexts_for_userid(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -114,7 +114,7 @@ class provider_test extends provider_testcase {
     /**
      * Test that user data is exported correctly.
      */
-    public function test_export_user_data() {
+    public function test_export_user_data(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -243,7 +243,7 @@ class provider_test extends provider_testcase {
     /**
      * Test for provider::delete_data_for_all_users_in_context().
      */
-    public function test_delete_data_for_all_users_in_context() {
+    public function test_delete_data_for_all_users_in_context(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -329,7 +329,7 @@ class provider_test extends provider_testcase {
     /**
      * Test for provider::delete_data_for_user().
      */
-    public function test_delete_data_for_user() {
+    public function test_delete_data_for_user(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -387,7 +387,7 @@ class provider_test extends provider_testcase {
     /**
      * Export for a user with a key against a script where no instance is specified.
      */
-    public function test_export_user_role_to_cohort() {
+    public function test_export_user_role_to_cohort(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -443,7 +443,7 @@ class provider_test extends provider_testcase {
     /**
      * Test for provider::delete_user_role_to_cohort().
      */
-    public function test_delete_user_role_to_cohort() {
+    public function test_delete_user_role_to_cohort(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -478,7 +478,7 @@ class provider_test extends provider_testcase {
     /**
      * Test that only users within a course context are fetched.
      */
-    public function test_get_users_in_context() {
+    public function test_get_users_in_context(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -599,7 +599,7 @@ class provider_test extends provider_testcase {
     /**
      * Test that data for users in approved userlist is deleted.
      */
-    public function test_delete_data_for_users() {
+    public function test_delete_data_for_users(): void {
         global $DB;
 
         $this->resetAfterTest();

admin/settings/security.php

@@ -152,7 +152,9 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page
     $sensiblesettingsdefault .= 'server_password@@search_solr, ssl_keypassword@@search_solr, ';
     $sensiblesettingsdefault .= 'alternateserver_password@@search_solr, alternatessl_keypassword@@search_solr, ';
     $sensiblesettingsdefault .= 'test_password@@cachestore_redis, password@@mlbackend_python, ';
-    $sensiblesettingsdefault .= 'badges_badgesalt@@none, calendar_exportsalt@@none';
+    $sensiblesettingsdefault .= 'badges_badgesalt@@none, calendar_exportsalt@@none, ';
+    $sensiblesettingsdefault .= 'bigbluebuttonbn_shared_secret@@none, apikey@@tiny_premium, ';
+    $sensiblesettingsdefault .= 'matrixaccesstoken@@communication_matrix, api_secret@@factor_sms';
     $temp->add(new admin_setting_configtextarea('adminpresets/sensiblesettings',
             get_string('sensiblesettings', 'core_adminpresets'),
             get_string('sensiblesettingstext', 'core_adminpresets'),

admin/templates/setting_configcheckbox.mustache

@@ -38,6 +38,6 @@
     }
 }}
 <div class="form-checkbox defaultsnext">
-    <input type="hidden" name="{{name}}" value="{{no}}">
+    {{^readonly}}<input type="hidden" name="{{name}}" value="{{no}}">{{/readonly}}
     <input {{#readonly}}disabled{{/readonly}} type="checkbox" name="{{name}}" value="{{value}}" id="{{id}}" {{#checked}}checked{{/checked}}>
 </div>

admin/templates/setting_configmulticheckbox.mustache

@@ -35,7 +35,7 @@
     }
 }}
 <div class="form-multicheckbox">
-    <input type="hidden" name="{{name}}[xxxxx]" value="1">
+    {{^readonly}}<input type="hidden" name="{{name}}[xxxxx]" value="1">{{/readonly}}
     {{#hasoptions}}
         <ul>
             {{#options}}

admin/templates/setting_configmultiselect.mustache

@@ -40,7 +40,7 @@
     Setting configmultiselect.
 }}
 <div class="form-select">
-    <input type="hidden" name="{{name}}[xxxxx]" value="1">
+    {{^readonly}}<input type="hidden" name="{{name}}[xxxxx]" value="1">{{/readonly}}
     <select {{#readonly}}disabled{{/readonly}} id="{{id}}" name="{{name}}[]" size="{{size}}" class="form-control" multiple>
         {{#options}}
             <option value="{{value}}" {{#selected}}selected{{/selected}}>{{name}}</option>

admin/templates/setting_configmultiselect_optgroup.mustache

@@ -59,7 +59,7 @@
     Setting configmultiselect with optgroup support.
 }}
 <div class="form-select">
-    <input type="hidden" name="{{name}}[xxxxx]" value="1">
+    {{^readonly}}<input type="hidden" name="{{name}}[xxxxx]" value="1">{{/readonly}}
     <select {{#readonly}}disabled{{/readonly}} id="{{id}}" name="{{name}}[]" size="{{size}}" class="form-control" multiple>
     {{#options}}
         <option value="{{value}}" {{#selected}}selected{{/selected}}>{{name}}</option>

admin/tests/behat/browse_users.feature

@@ -18,7 +18,7 @@ Feature: An administrator can browse user accounts
     When I navigate to "Users > Accounts > Browse list of users" in site administration
     # Name field always present, email field is default for showidentity.
     Then the following should exist in the "reportbuilder-table" table:
-      | First name / Last name | Email address   |
+      | First name           | Email address   |
       | User One             | one@example.com |
       | User Two             | two@example.com |
     # Should not see other identity fields or non-default name fields.
@@ -34,7 +34,7 @@ Feature: An administrator can browse user accounts
       | alternativefullnameformat | firstnamephonetic lastname |
     When I navigate to "Users > Accounts > Browse list of users" in site administration
     Then the following should exist in the "reportbuilder-table" table:
-      | First name - phonetic / Last name | Email address   |
+      | First name - phonetic           | Email address   |
       | Yewzer One                      | one@example.com |
       | Yoozare Two                     | two@example.com |
 
@@ -43,7 +43,7 @@ Feature: An administrator can browse user accounts
       | showuseridentity | department,profile_field_frog |
     When I navigate to "Users > Accounts > Browse list of users" in site administration
     Then the following should exist in the "reportbuilder-table" table:
-      | First name / Last name | Favourite frog  | Department |
+      | First name           | Favourite frog  | Department |
       | User One             | Kermit          | Attack     |
       | User Two             | Tree            | Defence    |
     And I should not see "Email address" in the "table" "css_element"
@@ -103,7 +103,7 @@ Feature: An administrator can browse user accounts
       | user3    | User      | Three    | three@example.com | Glass              |
     And I navigate to "Users > Accounts > Browse list of users" in site administration
     Then the following should exist in the "reportbuilder-table" table:
-      | First name / Last name | Email address     |
+      | First name             | Email address     |
       | User One               | one@example.com   |
       | User Two               | two@example.com   |
       | User Three             | three@example.com |
@@ -137,7 +137,7 @@ Feature: An administrator can browse user accounts
       | user2 | C1     | student |
     And I navigate to "Users > Accounts > Browse list of users" in site administration
     Then the following should exist in the "reportbuilder-table" table:
-      | First name / Last name | Email address     |
+      | First name             | Email address     |
       | User One               | one@example.com   |
       | User Two               | two@example.com   |
     And I click on "Filters" "button"
@@ -166,7 +166,7 @@ Feature: An administrator can browse user accounts
       | user1 | coursecreator | system       |           |
     And I navigate to "Users > Accounts > Browse list of users" in site administration
     Then the following should exist in the "reportbuilder-table" table:
-      | First name / Last name | Email address     |
+      | First name             | Email address     |
       | User One               | one@example.com   |
       | User Two               | two@example.com   |
       | User Three             | three@example.com |

admin/tests/behat/filter_users.feature

@@ -159,3 +159,24 @@ Feature: An administrator can filter user accounts by role, cohort and other pro
     And I should not see "User One" in the "reportbuilder-table" "table"
     And I should not see "User Three" in the "reportbuilder-table" "table"
     And I should not see "User Four" in the "reportbuilder-table" "table"
+
+  @javascript
+  Scenario: Filter users by full name
+    When I click on "Filters" "button"
+    And I set the following fields in the "Full name" "core_reportbuilder > Filter" to these values:
+      | Full name operator | Is equal to |
+      | Full name value    | User One    |
+    And I click on "Apply" "button" in the "[data-region='report-filters']" "css_element"
+    Then I should see "User One" in the "reportbuilder-table" "table"
+    And I should not see "User Two" in the "reportbuilder-table" "table"
+    And I should not see "User Three" in the "reportbuilder-table" "table"
+    And I should not see "User Four" in the "reportbuilder-table" "table"
+    And I click on "Reset all" "button" in the "[data-region='report-filters']" "css_element"
+    And I set the following fields in the "Full name" "core_reportbuilder > Filter" to these values:
+      | Full name operator | Is equal to |
+      | Full name value    | User Two    |
+    And I click on "Apply" "button" in the "[data-region='report-filters']" "css_element"
+    And I should see "User Two" in the "reportbuilder-table" "table"
+    And I should not see "User One" in the "reportbuilder-table" "table"
+    And I should not see "User Three" in the "reportbuilder-table" "table"
+    And I should not see "User Four" in the "reportbuilder-table" "table"

admin/tests/behat/task_logs.feature

@@ -48,25 +48,3 @@ Feature: View task logs report and use its filters
       | operator     | shouldornotsee |
       | Less than    | should not see |
       | Greater than | should see     |
-
-  @javascript
-  Scenario: Reset task log filters
-    Given I log in as "admin"
-    And I change window size to "large"
-    And I navigate to "Server > Tasks > Task logs" in site administration
-    When I click on "Filters" "button"
-    And I set the following fields in the "Result" "core_reportbuilder > Filter" to these values:
-      | Result operator | Is equal to |
-      | Result value    | Fail        |
-    And I click on "Apply" "button" in the "[data-region='report-filters']" "css_element"
-    Then I should see "Filters applied"
-    And I should see "Filters (1)" in the "#dropdownFiltersButton" "css_element"
-    And I should see "Nothing to display"
-    And I click on "Reset all" "button" in the "[data-region='report-filters']" "css_element"
-    And I should see "Filters reset"
-    And I should not see "Filters (1)" in the "#dropdownFiltersButton" "css_element"
-    And I should see "Filters" in the "#dropdownFiltersButton" "css_element"
-    And "[data-region='report-filters']" "css_element" should be visible
-    And the following fields in the "Result" "core_reportbuilder > Filter" match these values:
-      | Result operator | Is any value |
-    And I should not see "Nothing to display"

admin/tool/admin_presets/tests/behat/download.feature

@@ -5,16 +5,18 @@ Feature: I can download a preset
       | name       |
       | Custom preset |
 
-  @javascript
   Scenario: Custom preset settings can be downloaded
     Given I log in as "admin"
     And I navigate to "Site admin presets" in site administration
     When I open the action menu in "Custom preset" "table_row"
-    Then following "Download" "link" in the "Custom preset" "table_row" should download between "0" and "5000" bytes
+    Then following "Download" in the "Custom preset" "table_row" should download a file that:
+      | Has mimetype                 | text/xml      |
+      | Contains text in xml element | Custom preset |
 
-  @javascript
   Scenario: Core preset settings can be downloaded
     Given I log in as "admin"
     And I navigate to "Site admin presets" in site administration
     When I open the action menu in "Starter" "table_row"
-    Then following "Download" "link" in the "Starter" "table_row" should download between "0" and "5000" bytes
+    Then following "Download" in the "Starter" "table_row" should download a file that:
+      | Has mimetype                 | text/xml |
+      | Contains text in xml element | Starter  |

admin/tool/admin_presets/tests/event/preset_deleted_test.php

@@ -30,7 +30,7 @@ class preset_deleted_test extends \advanced_testcase {
     /**
      * Test preset_deleted event.
      */
-    public function test_preset_deleted_event() {
+    public function test_preset_deleted_event(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/admin_presets/tests/event/preset_downloaded_test.php

@@ -30,7 +30,7 @@ class preset_downloaded_test extends \advanced_testcase {
     /**
      * Test preset_downloaded event.
      */
-    public function test_preset_downloaded_event() {
+    public function test_preset_downloaded_event(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/admin_presets/tests/event/preset_exported_test.php

@@ -30,7 +30,7 @@ class preset_exported_test extends \advanced_testcase {
     /**
      * Test preset_exported event.
      */
-    public function test_preset_exported_event() {
+    public function test_preset_exported_event(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/admin_presets/tests/event/preset_imported_test.php

@@ -30,7 +30,7 @@ class preset_imported_test extends \advanced_testcase {
     /**
      * Test preset_imported event.
      */
-    public function test_preset_imported_event() {
+    public function test_preset_imported_event(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/admin_presets/tests/event/preset_loaded_test.php

@@ -30,7 +30,7 @@ class preset_loaded_test extends \advanced_testcase {
     /**
      * Test preset_loaded event.
      */
-    public function test_preset_loaded_event() {
+    public function test_preset_loaded_event(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/admin_presets/tests/event/preset_previewed_test.php

@@ -30,7 +30,7 @@ class preset_previewed_test extends \advanced_testcase {
     /**
      * Test preset_previewed event.
      */
-    public function test_preset_previewed_event() {
+    public function test_preset_previewed_event(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/admin_presets/tests/event/preset_reverted_test.php

@@ -30,7 +30,7 @@ class preset_reverted_test extends \advanced_testcase {
     /**
      * Test preset_reverted event.
      */
-    public function test_preset_reverted_event() {
+    public function test_preset_reverted_event(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/admin_presets/tests/event/presets_listed_test.php

@@ -30,7 +30,7 @@ class presets_listed_test extends \advanced_testcase {
     /**
      * Test presets_listed event.
      */
-    public function test_presets_listed_event() {
+    public function test_presets_listed_event(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/analytics/tests/external/external_test.php

@@ -50,7 +50,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * test_potential_contexts description
      */
-    public function test_potential_contexts() {
+    public function test_potential_contexts(): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -71,7 +71,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * test_potential_contexts description
      */
-    public function test_potential_contexts_no_manager() {
+    public function test_potential_contexts_no_manager(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user();

admin/tool/behat/tests/behat/tabs.feature

@@ -25,7 +25,7 @@ Feature: Confirm that we can open multiple browser tabs
     And I open a tab named "CourseViewer4" on the "My courses" page
 
     # Switch between all the tabs and confirm their different contents.
-    Then I should see "You're not enrolled in any course"
+    Then I should see "You're not enrolled in any courses."
     And I switch to "CourseViewer2" tab
     And "Course 3" "heading" should exist
     And I switch to "CourseViewer1" tab

admin/tool/behat/tests/behat_form_text_test.php

@@ -70,7 +70,7 @@ class behat_form_text_test extends \basic_testcase {
      * @param mixed $expectation value to be checked.
      * @dataProvider provider_test_set_get_value()
      */
-    public function test_set_get_value($value, $expectation) {
+    public function test_set_get_value($value, $expectation): void {
         $session = $this->createMock(Session::class);
         $node = $this->createMock(NodeElement::class);
         $node->method('getValue')->willReturn($value);
@@ -115,7 +115,7 @@ class behat_form_text_test extends \basic_testcase {
      * @param bool  $result expected return status of the function.
      * @dataProvider provider_test_matches()
      */
-    public function test_matches($decsep, $value, $match, $result) {
+    public function test_matches($decsep, $value, $match, $result): void {
         global $CFG;
 
         // Switch of string manager to avoid having to (slow) customise the lang file.

admin/tool/behat/tests/manager_util_test.php

@@ -25,6 +25,10 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
+namespace tool_behat;
+
+use behat_config_util;
+
 defined('MOODLE_INTERNAL') || die();
 
 global $CFG;
@@ -39,7 +43,7 @@ require_once($CFG->libdir . '/behat/classes/behat_config_manager.php');
  * @copyright  2016 Rajesh Taneja
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class tool_behat_manager_util_testcase extends advanced_testcase {
+class manager_util_test extends \advanced_testcase {
 
     /** @var array Fixtures features which are available. */
     private $featurepaths = array(

admin/tool/brickfield/classes/local/htmlchecker/common/brickfield_accessibility_color_test.php

@@ -245,7 +245,6 @@ class brickfield_accessibility_color_test extends brickfield_accessibility_test
         return $luminosity;
     }
 
-
     /**
      * Returns the decimal equivalents for a HEX color. Returns null if it cannot be determined.
      * @param string $color The hex color value
@@ -268,13 +267,21 @@ class brickfield_accessibility_color_test extends brickfield_accessibility_test
      */
     public function convert_color(string $color): string {
         $color = trim($color);
-        if (strpos($color, ' ') !== false) {
-            $colors = explode(' ', $color);
-            foreach ($colors as $backgroundpart) {
-                if (substr(trim($backgroundpart), 0, 1) == '#' ||
-                    in_array(trim($backgroundpart), array_keys($this->colornames)) ||
-                    strtolower(substr(trim($backgroundpart), 0, 3)) == 'rgb') {
-                    $color = $backgroundpart;
+        // Search for color in rgb format first, as this can potentially contain a space.
+        if (strpos($color, 'rgb') !== false) {
+            $colors = explode('rgb', $color, 2); // Getting 2 only in array.
+            // Getting end point of rgb value, i.e. the end bracket.
+            $endpos = strpos($colors[1], ')');
+            $color = 'rgb' . substr($colors[1], 0, ($endpos + 1)); // Recompiling rgb value.
+        } else {
+            // Splitting multi-value css background value.
+            if (strpos($color, ' ') !== false) {
+                $colors = explode(' ', $color);
+                foreach ($colors as $backgroundpart) {
+                    if (substr(trim($backgroundpart), 0, 1) == '#' ||
+                        in_array(trim($backgroundpart), array_keys($this->colornames))) {
+                        $color = $backgroundpart;
+                    }
                 }
             }
         }
@@ -296,9 +303,20 @@ class brickfield_accessibility_color_test extends brickfield_accessibility_test
         }
         // RGB values.
         if (strtolower(substr($color, 0, 3)) == 'rgb') {
-            $colors = explode(',', trim(str_replace('rgb(', '', $color), '()'));
-            if (count($colors) != 3) {
-                return false;
+            if (strpos($color, 'rgba') !== false) {
+                $tmpbg = $this->get_rgb($this->defaultbackground);
+                $colors = explode(',', trim(str_replace('rgba(', '', $color), '()'));
+                if (count($colors) != 4) {
+                    return false;
+                }
+                $colors[0] = round(((1 - $colors[3]) * $tmpbg['r']) + ($colors[3] * $colors[0]));
+                $colors[1] = round((1 - $colors[3]) * $tmpbg['g']) + ($colors[3] * $colors[1]);
+                $colors[2] = round((1 - $colors[3]) * $tmpbg['b']) + ($colors[3] * $colors[2]);
+            } else {
+                $colors = explode(',', trim(str_replace('rgb(', '', $color), '()'));
+                if (count($colors) != 3) {
+                    return false;
+                }
             }
             $r = intval($colors[0]);
             $g = intval($colors[1]);

admin/tool/brickfield/classes/local/htmlchecker/common/checks/css_text_has_contrast.php

@@ -62,9 +62,18 @@ class css_text_has_contrast extends brickfield_accessibility_color_test {
         foreach ($entries as $element) {
             $style = $this->css->get_style($element);
 
-            if (isset($style['background-color']) || isset($style['color'])) {
+            if (isset($style['background-color']) || isset($style['color']) || isset($style['background'])) {
                 if (!isset($style['background-color'])) {
-                    $style['background-color'] = $this->defaultbackground;
+                    if (isset($style['background'])) {
+                        // Parsing background-color from CSS background shortcut string.
+                        $style['background-color'] = '#' . $this->convert_color($style['background']);
+                        // If value is empty after hash, then use defaultbackground.
+                        if ($style['background-color'] == '#') {
+                            $style['background-color'] = $this->defaultbackground;
+                        }
+                    } else {
+                        $style['background-color'] = $this->defaultbackground;
+                    }
                 }
 
                 if (!isset($style['color'])) {
@@ -82,11 +91,7 @@ class css_text_has_contrast extends brickfield_accessibility_color_test {
                     $style['color'] = '#' . $this->convert_color($style['color']);
                     $style['background-color'] = '#' . $this->convert_color($background);
 
-                    if (substr($background, 0, 3) == "rgb") {
-                        $background = '#' . $this->convert_color($background);
-                    }
-
-                    $luminosity = $this->get_luminosity($style['color'], $background);
+                    $luminosity = $this->get_luminosity($style['color'], $style['background-color']);
                     $fontsize = 0;
                     $bold = false;
                     $italic = false;

admin/tool/brickfield/tests/accessibility_test.php

@@ -35,7 +35,7 @@ class accessibility_test extends \advanced_testcase {
      * @throws \dml_exception
      * @throws \moodle_exception
      */
-    public function test_get_title() {
+    public function test_get_title(): void {
         $this->resetAfterTest();
         $object = new accessibility();
         $filter = new filter();
@@ -57,7 +57,7 @@ class accessibility_test extends \advanced_testcase {
      *
      * @throws \dml_exception
      */
-    public function test_check_ids() {
+    public function test_check_ids(): void {
         $this->resetAfterTest();
         $object = new accessibility();
 
@@ -74,7 +74,7 @@ class accessibility_test extends \advanced_testcase {
      *
      * @throws \dml_exception
      */
-    public function test_get_translations() {
+    public function test_get_translations(): void {
         $this->resetAfterTest();
         $object = new accessibility();
 
@@ -90,7 +90,7 @@ class accessibility_test extends \advanced_testcase {
      *
      * @throws \dml_exception
      */
-    public function test_get_category_courseids() {
+    public function test_get_category_courseids(): void {
         $this->resetAfterTest();
         $object = new accessibility();
         $category = $this->getDataGenerator()->create_category();

admin/tool/brickfield/tests/area_test.php

@@ -32,7 +32,7 @@ class area_test extends area_test_base {
     /**
      * Test for the area assign intro
      */
-    public function test_assign() {
+    public function test_assign(): void {
         $this->resetAfterTest();
         $course = $this->getDataGenerator()->create_course();
         $assign1 = $this->getDataGenerator()->create_module('assign', array(
@@ -87,7 +87,7 @@ class area_test extends area_test_base {
     /**
      * Test for the area questiontext
      */
-    public function test_questiontext() {
+    public function test_questiontext(): void {
         $this->resetAfterTest();
         /** @var \core_question_generator $generator */
         $generator = $this->getDataGenerator()->get_plugin_generator('core_question');
@@ -147,7 +147,7 @@ class area_test extends area_test_base {
     /**
      * test for the area questionanswers
      */
-    public function test_questionanswers() {
+    public function test_questionanswers(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -191,7 +191,7 @@ class area_test extends area_test_base {
     /**
      * Test for the areas choice intro and choice options
      */
-    public function test_choice() {
+    public function test_choice(): void {
         global $DB;
         $this->resetAfterTest();
         $course = $this->getDataGenerator()->create_course();

admin/tool/brickfield/tests/filters_test.php

@@ -27,7 +27,7 @@ use tool_brickfield\local\tool\filter;
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class filters_test extends \advanced_testcase {
-    public function test_constructor() {
+    public function test_constructor(): void {
         $this->resetAfterTest();
 
         // Variables.
@@ -56,7 +56,7 @@ class filters_test extends \advanced_testcase {
         $this->assertEquals($object->url, $url);
     }
 
-    public function test_get_course_sql() {
+    public function test_get_course_sql(): void {
         $this->resetAfterTest();
         $object = new filter();
 
@@ -73,7 +73,7 @@ class filters_test extends \advanced_testcase {
 
     }
 
-    public function test_validate_filters() {
+    public function test_validate_filters(): void {
         $this->resetAfterTest();
         // Variables.
         $courseid = 0;
@@ -102,7 +102,7 @@ class filters_test extends \advanced_testcase {
         $this->assertFalse($output);
     }
 
-    public function test_has_course_filters() {
+    public function test_has_course_filters(): void {
         $this->resetAfterTest();
 
         $object = new filter();
@@ -114,7 +114,7 @@ class filters_test extends \advanced_testcase {
         $this->assertTrue($output);
     }
 
-    public function test_has_capability_in_context() {
+    public function test_has_capability_in_context(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -151,7 +151,7 @@ class filters_test extends \advanced_testcase {
         $this->assertTrue($output);
     }
 
-    public function test_get_errormessage() {
+    public function test_get_errormessage(): void {
         $this->resetAfterTest();
         // Variables.
         $courseid = 0;

admin/tool/brickfield/tests/local/areas/core_question/questionanswers_test.php

@@ -36,7 +36,7 @@ class questionanswers_test extends \advanced_testcase {
     /**
      * Test find course areas.
      */
-    public function test_find_course_areas() {
+    public function test_find_course_areas(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -122,7 +122,7 @@ class questionanswers_test extends \advanced_testcase {
     /**
      * Test find relevant areas.
      */
-    public function test_find_relevant_areas() {
+    public function test_find_relevant_areas(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -153,7 +153,7 @@ class questionanswers_test extends \advanced_testcase {
     /**
      * Test find system areas.
      */
-    public function test_find_system_areas() {
+    public function test_find_system_areas(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/brickfield/tests/local/areas/core_question/questiontext_test.php

@@ -42,7 +42,7 @@ class questiontext_test extends area_test_base {
     /**
      * Test find course areas.
      */
-    public function test_find_course_areas() {
+    public function test_find_course_areas(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -126,7 +126,7 @@ class questiontext_test extends area_test_base {
     /**
      * Test find relevant areas.
      */
-    public function test_find_relevant_areas() {
+    public function test_find_relevant_areas(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -156,7 +156,7 @@ class questiontext_test extends area_test_base {
     /**
      * Test find system areas.
      */
-    public function test_find_system_areas() {
+    public function test_find_system_areas(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -197,7 +197,7 @@ class questiontext_test extends area_test_base {
      *
      * @covers ::get_course_and_category
      */
-    public function test_get_course_and_category() {
+    public function test_get_course_and_category(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/a_links_dont_open_new_window_test.php

@@ -103,7 +103,7 @@ EOD;
     /**
      * Test for links opening a new tab or window
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'a');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/a_must_contain_text_test.php

@@ -64,7 +64,7 @@ EOD;
     /**
      * Test for anchor tags containing text
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'a');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/a_suspicious_link_text_test.php

@@ -90,7 +90,7 @@ EOD;
     /**
      * Test for anchor tags not containing susplicious link text
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlpass);
         $this->assertEmpty($results);
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/area_dont_open_new_window_test.php

@@ -103,7 +103,7 @@ EOD;
     /**
      * Test Area tags opening new window without warning
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'area');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/area_has_alt_value_test.php

@@ -64,7 +64,7 @@ EOD;
     /**
      * Test for area tags containing an alt value
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'area');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/basefont_is_not_used_test.php

@@ -64,7 +64,7 @@ EOD;
     /**
      * Test for basefont tags being used
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'basefont');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/blink_is_not_used_test.php

@@ -64,7 +64,7 @@ EOD;
     /**
      * Test for blink tags being used
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'blink');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/bold_is_not_used_test.php

@@ -65,7 +65,7 @@ EOD;
     /**
      * Test for b tags being used
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'b');
         $this->assertTrue($results[1]->element->tagName == 'b');

admin/tool/brickfield/tests/local/htmlchecker/common/checks/content_too_long_test.php

@@ -124,7 +124,7 @@ EOD;
     /**
      * Test for checking the length of the content
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->message == '<p id=\'wc\'>Word Count: 578</p>');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/css_text_has_contrast_test.php

@@ -30,6 +30,7 @@ require_once('all_checks.php');
 
 /**
  * Class test_css_text_has_contrast_test
+ * @covers \tool_brickfield\local\htmlchecker\brickfield_accessibility
  */
 class css_text_has_contrast_test extends all_checks {
     /** @var string The check type. */
@@ -229,7 +230,7 @@ EOD;
     /**
      * Test for the area assign intro
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail1);
         $this->assertTrue($results[0]->element->tagName == 'p');
 
@@ -255,7 +256,7 @@ EOD;
     /**
      * Test with valid colour names.
      */
-    public function test_check_for_namedcolours() {
+    public function test_check_for_namedcolours(): void {
         $results = $this->get_checker_results($this->namecolours);
         $this->assertTrue($results[0]->element->tagName == 'p');
     }
@@ -263,7 +264,7 @@ EOD;
     /**
      * Test with invalid colour names.
      */
-    public function test_check_for_invalidcolours() {
+    public function test_check_for_invalidcolours(): void {
         $results = $this->get_checker_results($this->invalidcolours);
         $this->assertTrue($results[0]->element->tagName == 'p');
     }
@@ -271,7 +272,7 @@ EOD;
     /**
      * Test with invalid colour numeric values.
      */
-    public function test_check_for_invalidvalues() {
+    public function test_check_for_invalidvalues(): void {
         $results = $this->get_checker_results($this->invalidvalue);
         $this->assertTrue($results[0]->element->tagName == 'p');
     }
@@ -279,7 +280,7 @@ EOD;
     /**
      * Test with empty colour values.
      */
-    public function test_check_for_emptyvalues() {
+    public function test_check_for_emptyvalues(): void {
         $results = $this->get_checker_results($this->emptyvalue);
         $this->assertEmpty($results);
     }
@@ -287,7 +288,7 @@ EOD;
     /**
      * Test for text px18 with insufficient contrast of 4.49.
      */
-    public function test_check_for_px18_fail() {
+    public function test_check_for_px18_fail(): void {
         $results = $this->get_checker_results($this->px18);
         $this->assertTrue($results[0]->element->tagName == 'p');
     }
@@ -295,7 +296,7 @@ EOD;
     /**
      * Test for text px19 bold with sufficient contrast of 4.49.
      */
-    public function test_check_for_px19bold_pass() {
+    public function test_check_for_px19bold_pass(): void {
         $results = $this->get_checker_results($this->px19bold);
         $this->assertEmpty($results);
     }
@@ -303,7 +304,7 @@ EOD;
     /**
      * Test for text px18 with sufficient contrast of 4.81.
      */
-    public function test_check_for_px18_pass() {
+    public function test_check_for_px18_pass(): void {
         $results = $this->get_checker_results($this->px18pass);
         $this->assertEmpty($results);
     }
@@ -311,7 +312,7 @@ EOD;
     /**
      * Test for medium (12pt) text with insufficient contrast of 4.49.
      */
-    public function test_check_for_medium_fail() {
+    public function test_check_for_medium_fail(): void {
         $results = $this->get_checker_results($this->mediumfail);
         $this->assertTrue($results[0]->element->tagName == 'p');
     }
@@ -319,7 +320,7 @@ EOD;
     /**
      * Test for medium (12pt) text with sufficient contrast of 4.81.
      */
-    public function test_check_for_medium_pass() {
+    public function test_check_for_medium_pass(): void {
         $results = $this->get_checker_results($this->mediumpass);
         $this->assertEmpty($results);
     }
@@ -327,7 +328,7 @@ EOD;
     /**
      * Test for larger (14pt) text with insufficient contrast of 2.94.
      */
-    public function test_check_for_larger_fail() {
+    public function test_check_for_larger_fail(): void {
         $results = $this->get_checker_results($this->largerfail);
         $this->assertTrue($results[0]->element->tagName == 'p');
     }
@@ -335,7 +336,7 @@ EOD;
     /**
      * Test for larger (14pt) text with insufficient contrast of 3.02.
      */
-    public function test_check_for_larger_pass() {
+    public function test_check_for_larger_pass(): void {
         $results = $this->get_checker_results($this->largerpass);
         $this->assertTrue($results[0]->element->tagName == 'p');
     }
@@ -343,8 +344,118 @@ EOD;
     /**
      * Test for larger (14pt) bold text with sufficient contrast of 3.02.
      */
-    public function test_check_for_largerbold_pass() {
+    public function test_check_for_largerbold_pass(): void {
         $results = $this->get_checker_results($this->largerboldpass);
         $this->assertEmpty($results);
     }
+
+    /**
+     * Test for rgb colors with insufficient contrast.
+     */
+    public function test_bad_rgbcolor(): void {
+        $html = '<body><p style="color:rgb(255, 255, 255); background-color:rgb(204, 204, 204);">
+            This is not contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertTrue($results[0]->element->tagName == 'p');
+    }
+
+    /**
+     * Test for rgb colors with sufficient contrast.
+     */
+    public function test_good_rgbcolor(): void {
+        $html = '<body><p style="color:rgb(255, 255, 255); background-color:rgb(0, 0, 0);">
+            This is contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertEmpty($results);
+    }
+
+    /**
+     * Test for named colors with insufficient contrast.
+     */
+    public function test_bad_namedcolor2(): void {
+        $html = '<body><p style="color:lightcyan; background-color:lavender;">
+            This is not contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertTrue($results[0]->element->tagName == 'p');
+    }
+
+    /**
+     * Test for named colors with sufficient contrast.
+     */
+    public function test_good_namedcolor2(): void {
+        $html = '<body><p style="color:linen; background-color:darkslategray;">
+            This is contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertEmpty($results);
+    }
+
+    /**
+     * Test for background value with insufficient contrast.
+     */
+    public function test_bad_backgroundcss(): void {
+        $html = '<body><p style="color:lightcyan; background:fixed lavender center;">
+            This is not contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertTrue($results[0]->element->tagName == 'p');
+    }
+
+    /**
+     * Test for background value with sufficient contrast.
+     */
+    public function test_good_backgroundcss(): void {
+        $html = '<body><p style="color:linen; background:fixed darkslategray center;">
+            This is contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertEmpty($results);
+    }
+
+    /**
+     * Test for background value with rgb with insufficient contrast.
+     */
+    public function test_bad_backgroundcssrgb(): void {
+        $html = '<body><p style="color:rgb(255, 255, 255); background:fixed rgb(204, 204, 204) center;">
+            This is not contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertTrue($results[0]->element->tagName == 'p');
+    }
+
+    /**
+     * Test for background value with rgb with sufficient contrast.
+     */
+    public function test_good_backgroundcssrgb(): void {
+        $html = '<body><p style="color:rgb(255, 255, 255); background:fixed rgb(0, 0, 0) center;">
+            This is contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertEmpty($results);
+    }
+
+    /**
+     * Test for text with insufficient contrast of 4.3.
+     */
+    public function test_bad_contrastrounding(): void {
+        $html = '<body><p style="color:#F50000; background-color:white; font-size: 12px">
+            This is not contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertTrue($results[0]->element->tagName == 'p');
+    }
+
+    /**
+     * Test for background value with rgba with insufficient contrast.
+     */
+    public function test_bad_backgroundcssrgba(): void {
+        $html = '<body><p style="color:rgba(255, 255, 255, 0.5); background:fixed rgba(0, 204, 204, 0.5) center;">
+            This is not contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertTrue($results[0]->element->tagName == 'p');
+    }
+
+    /**
+     * Test for background value with rgba with sufficient contrast.
+     */
+    public function test_good_backgroundcssrgba(): void {
+        $html = '<body><p style="color:rgba(255, 255, 255, 0.75); background:fixed rgba(0, 0, 0, 0.75) center;">
+            This is contrasty enough.</p></body>';
+        $results = $this->get_checker_results($html);
+        $this->assertEmpty($results);
+    }
 }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/embed_has_associated_no_embed_test.php

@@ -81,7 +81,7 @@ EOD;
     /**
      * Test for noembed child tags
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'embed');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/header_h3_test.php

@@ -122,7 +122,7 @@ EOD;
     /**
      * Test the header following an h3 must be h1, h2, h3 or h4.
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail1);
         $this->assertTrue($results[0]->element->tagName == 'h5');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/headers_have_text_test.php

@@ -76,7 +76,7 @@ EOD;
     /**
      * Test for headers not containing text
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'h2');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/i_is_not_used_test.php

@@ -64,7 +64,7 @@ EOD;
     /**
      * Test for i tags being used
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'i');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/img_alt_is_different_test.php

@@ -64,7 +64,7 @@ EOD;
     /**
      * Test for img alt attributes matching the src attribute
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'img');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/img_alt_is_too_long_test.php

@@ -68,7 +68,7 @@ EOD;
     /**
      * Test for image alt attributes being too long
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'img');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/img_alt_not_empty_in_anchor_test.php

@@ -63,7 +63,7 @@ EOD;
     /**
      * Test for >Anchor tags containing a nested img tag, must not have an empty alt attribute
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'img');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/img_alt_not_place_holder_test.php

@@ -133,7 +133,7 @@ EOD;
     /**
      * Test for the each en string specified in $strings - line 43 in imgAltNotPlaceHolder.
      */
-    public function test_failcheck() {
+    public function test_failcheck(): void {
         $results = $this->get_checker_results($this->htmlfail1);
         $this->assertTrue($results[0]->element->tagName == 'img');
 
@@ -156,7 +156,7 @@ EOD;
     /**
      * Test with alt that was not specified in the $strings array.
      */
-    public function test_passcheck1() {
+    public function test_passcheck1(): void {
         $results = $this->get_checker_results($this->htmlpass1);
         $this->assertEmpty($results);
     }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/img_has_alt_test.php

@@ -76,7 +76,7 @@ EOD;
     /**
      * Test for missing image alt text.
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail1);
         $this->assertTrue($results[0]->element->tagName == 'img');
 
@@ -87,7 +87,7 @@ EOD;
     /**
      * Test for present image alt text.
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass);
         $this->assertTrue(count($results) == 0);
     }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/img_with_map_has_use_map_test.php

@@ -66,7 +66,7 @@ EOD;
     /**
      * Test for usemap attribute.
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'img');
     }
@@ -74,7 +74,7 @@ EOD;
     /**
      * Test for ismap attribute.
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass);
         $this->assertEmpty($results);
     }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/legend_text_not_empty_test.php

@@ -76,7 +76,7 @@ EOD;
     /**
      * Test for if legend is empty or only has white spaces.
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail1);
         $this->assertTrue($results[0]->element->tagName == 'legend');
 
@@ -87,7 +87,7 @@ EOD;
     /**
      * Test for if legend has text.
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass);
         $this->assertEmpty($results);
     }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/no_headings_test.php

@@ -254,7 +254,7 @@ EOD;
     /**
      * Test for if heading exists where doc length is above 1800
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertNotEmpty($results);
 
@@ -263,7 +263,7 @@ EOD;
     /**
      * Test for if marquee does not exist.
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass1);
         $this->assertEmpty($results);
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/object_must_have_embed_test.php

@@ -66,7 +66,7 @@ EOD;
     /**
      * Test for missing embed element within object element.
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'object');
     }
@@ -74,7 +74,7 @@ EOD;
     /**
      * Test for present embed element within object element.
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass);
         $this->assertEmpty($results);
     }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/object_must_have_title_test.php

@@ -63,7 +63,7 @@ EOD;
     /**
      * Test for missing title attribute within object element.
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'object');
     }
@@ -71,7 +71,7 @@ EOD;
     /**
      * Test for present title attribute within object element.
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass);
         $this->assertEmpty($results);
     }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/object_must_have_valid_title_test.php

@@ -129,7 +129,7 @@ EOD;
     /**
      * Test that embed element within object element.
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'object');
     }
@@ -137,7 +137,7 @@ EOD;
     /**
      * Test for embed element within object element.
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass1);
         $this->assertEmpty($results);
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/strike_is_not_used_test.php

@@ -77,7 +77,7 @@ EOD;
     /**
      * Test for b tags being used
      */
-    public function test_check() {
+    public function test_check(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'strike');
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/table_data_should_have_th_test.php

@@ -117,7 +117,7 @@ EOD;
     /**
      * Test that th does not exist
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail1);
         $this->assertTrue($results[0]->element->tagName == 'table');
 
@@ -128,7 +128,7 @@ EOD;
     /**
      * Test that th does exist
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass1);
         $this->assertEmpty($results);
 

admin/tool/brickfield/tests/local/htmlchecker/common/checks/table_summary_does_not_duplicate_caption_test.php

@@ -67,7 +67,7 @@ EOD;
     /**
      * Test that caption and summary are equal
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail);
         $this->assertTrue($results[0]->element->tagName == 'table');
     }
@@ -75,7 +75,7 @@ EOD;
     /**
      * Test that caption and summary are not equal
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass);
         $this->assertEmpty($results);
     }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/table_td_should_not_merge_test.php

@@ -125,7 +125,7 @@ EOD;
     /**
      * Test for rowspan and colspan
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail1);
         $this->assertNotEmpty($results);
 
@@ -136,7 +136,7 @@ EOD;
     /**
      * Test for rowspan and colspan
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass);
         $this->assertEmpty($results);
     }

admin/tool/brickfield/tests/local/htmlchecker/common/checks/table_th_should_have_scope_test.php

@@ -163,7 +163,7 @@ EOD;
     /**
      * Test that th has scope that is equal to col or row
      */
-    public function test_check_fail() {
+    public function test_check_fail(): void {
         $results = $this->get_checker_results($this->htmlfail1);
         $this->assertEquals(2, count($results));
         $this->assertTrue($results[0]->element->tagName == 'th');
@@ -177,7 +177,7 @@ EOD;
     /**
      * Test that th has scope but != col || row. Test that th has no scope
      */
-    public function test_check_pass() {
+    public function test_check_pass(): void {
         $results = $this->get_checker_results($this->htmlpass1);
         $this->assertEmpty($results);
 

admin/tool/brickfield/tests/local/tool/activityresults_test.php

@@ -26,7 +26,7 @@ namespace tool_brickfield\local\tool;
  */
 class activityresults_test extends \advanced_testcase {
 
-    public function test_toolname() {
+    public function test_toolname(): void {
         $this->resetAfterTest();
 
         $object = new activityresults();
@@ -34,7 +34,7 @@ class activityresults_test extends \advanced_testcase {
         $this->assertEquals($output, 'Activity breakdown summary');
     }
 
-    public function test_toolshortname() {
+    public function test_toolshortname(): void {
         $this->resetAfterTest();
 
         $object = new activityresults();
@@ -42,7 +42,7 @@ class activityresults_test extends \advanced_testcase {
         $this->assertEquals($output, 'Activity breakdown');
     }
 
-    public function test_pluginname() {
+    public function test_pluginname(): void {
         $this->resetAfterTest();
 
         $object = new activityresults();
@@ -50,7 +50,7 @@ class activityresults_test extends \advanced_testcase {
         $this->assertEquals($output, 'activityresults');
     }
 
-    public function test_get_output() {
+    public function test_get_output(): void {
         $this->resetAfterTest();
 
         $category = $this->getDataGenerator()->create_category();

admin/tool/brickfield/tests/local/tool/checktyperesults_test.php

@@ -26,7 +26,7 @@ namespace tool_brickfield\local\tool;
  */
 class checktyperesults_test extends \advanced_testcase {
 
-    public function test_toolname() {
+    public function test_toolname(): void {
         $this->resetAfterTest();
 
         $object = new checktyperesults();
@@ -34,7 +34,7 @@ class checktyperesults_test extends \advanced_testcase {
         $this->assertEquals($output, 'Content types summary');
     }
 
-    public function test_toolshortname() {
+    public function test_toolshortname(): void {
         $this->resetAfterTest();
 
         $object = new checktyperesults();
@@ -42,7 +42,7 @@ class checktyperesults_test extends \advanced_testcase {
         $this->assertEquals($output, 'Content types');
     }
 
-    public function test_pluginname() {
+    public function test_pluginname(): void {
         $this->resetAfterTest();
 
         $object = new checktyperesults();
@@ -50,7 +50,7 @@ class checktyperesults_test extends \advanced_testcase {
         $this->assertEquals($output, 'checktyperesults');
     }
 
-    public function test_get_output() {
+    public function test_get_output(): void {
         $this->resetAfterTest();
         $category = $this->getDataGenerator()->create_category();
 

admin/tool/brickfield/tests/local/tool/errors_test.php

@@ -26,7 +26,7 @@ namespace tool_brickfield\local\tool;
  */
 class errors_test extends \advanced_testcase {
 
-    public function test_toolname() {
+    public function test_toolname(): void {
         $this->resetAfterTest();
 
         $object = new errors();
@@ -34,7 +34,7 @@ class errors_test extends \advanced_testcase {
         $this->assertEquals($output, 'Error list summary');
     }
 
-    public function test_toolshortname() {
+    public function test_toolshortname(): void {
         $this->resetAfterTest();
 
         $object = new errors();
@@ -42,7 +42,7 @@ class errors_test extends \advanced_testcase {
         $this->assertEquals($output, 'Error list');
     }
 
-    public function test_pluginname() {
+    public function test_pluginname(): void {
         $this->resetAfterTest();
 
         $object = new errors();
@@ -50,7 +50,7 @@ class errors_test extends \advanced_testcase {
         $this->assertEquals($output, 'errors');
     }
 
-    public function test_get_output() {
+    public function test_get_output(): void {
         $this->resetAfterTest();
         $category = $this->getDataGenerator()->create_category();
 

admin/tool/brickfield/tests/local/tool/printable_test.php

@@ -29,7 +29,7 @@ class printable_test extends \advanced_testcase {
     /**
      * Test tool name.
      */
-    public function test_toolname() {
+    public function test_toolname(): void {
         $this->resetAfterTest();
 
         $object = new printable();
@@ -40,7 +40,7 @@ class printable_test extends \advanced_testcase {
     /**
      * Test tool short name.
      */
-    public function test_toolshortname() {
+    public function test_toolshortname(): void {
         $this->resetAfterTest();
 
         $object = new printable();
@@ -51,7 +51,7 @@ class printable_test extends \advanced_testcase {
     /**
      * Test plugin name.
      */
-    public function test_pluginname() {
+    public function test_pluginname(): void {
         $this->resetAfterTest();
 
         $object = new printable();
@@ -77,7 +77,7 @@ class printable_test extends \advanced_testcase {
     /**
      * Test get output.
      */
-    public function test_get_output() {
+    public function test_get_output(): void {
         $this->resetAfterTest();
         $category = $this->getDataGenerator()->create_category();
 

admin/tool/brickfield/tests/manager_test.php

@@ -28,7 +28,7 @@ class manager_test extends \advanced_testcase {
     /**
      * Tests for the function manager::get_all_areas()
      */
-    public function test_get_areas() {
+    public function test_get_areas(): void {
         $this->resetAfterTest();
         $areas = manager::get_all_areas();
         $areaclassnames = array_map('get_class', $areas);

admin/tool/brickfield/tests/registration_test.php

@@ -40,7 +40,7 @@ class registration_test extends \advanced_testcase {
      * Tests the state of the registration system when first installed.
      * @throws \dml_exception
      */
-    public function test_initial_state() {
+    public function test_initial_state(): void {
         $this->resetAfterTest();
         $regobj = new mock_registration();
 
@@ -56,7 +56,7 @@ class registration_test extends \advanced_testcase {
      * Test the various states for setting registration keys.
      * @throws \dml_exception
      */
-    public function test_set_keys_for_registration() {
+    public function test_set_keys_for_registration(): void {
         $this->resetAfterTest();
         $regobj = new mock_registration();
 
@@ -79,7 +79,7 @@ class registration_test extends \advanced_testcase {
      * Test the validation system through its several states.
      * @throws \dml_exception
      */
-    public function test_validation() {
+    public function test_validation(): void {
         $this->resetAfterTest();
         $regobj = new mock_registration();
 
@@ -112,7 +112,7 @@ class registration_test extends \advanced_testcase {
      * Tests the system after validation grace periods expire.
      * @throws \dml_exception
      */
-    public function test_validation_time_expiry() {
+    public function test_validation_time_expiry(): void {
         $this->resetAfterTest();
         $regobj = new mock_registration();
 
@@ -149,7 +149,7 @@ class registration_test extends \advanced_testcase {
      * Tests the system after summary data time periods expire.
      * @throws \dml_exception
      */
-    public function test_summary_time_expiry() {
+    public function test_summary_time_expiry(): void {
         $this->resetAfterTest();
         $regobj = new mock_registration();
 

admin/tool/brickfield/tests/scheduler_test.php

@@ -26,7 +26,7 @@ namespace tool_brickfield;
  */
 class scheduler_test extends \advanced_testcase {
 
-    public function test_request_analysis() {
+    public function test_request_analysis(): void {
         $this->resetAfterTest();
 
         // I believe there is a bug where the code won't work with the default constructor values.
@@ -48,7 +48,7 @@ class scheduler_test extends \advanced_testcase {
 
     }
 
-    public function test_mark_analyzed() {
+    public function test_mark_analyzed(): void {
         $this->resetAfterTest();
         $object = new scheduler();
         $output = $object->mark_analyzed();
@@ -59,7 +59,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertTrue($output);
     }
 
-    public function test_create_schedule() {
+    public function test_create_schedule(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -76,7 +76,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertEquals($record->instanceid, 1);
     }
 
-    public function test_delete_schedule() {
+    public function test_delete_schedule(): void {
         global $DB;
 
         // Call create_record() to insert a record into the table.
@@ -95,7 +95,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertFalse($record);
     }
 
-    public function test_is_in_schedule() {
+    public function test_is_in_schedule(): void {
         $this->resetAfterTest();
 
         // This should assert to false as no record has been inserted.
@@ -109,7 +109,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertTrue($output);
     }
 
-    public function test_is_scheduled() {
+    public function test_is_scheduled(): void {
         $this->resetAfterTest();
 
         // This should assert to false as no record has been inserted.
@@ -129,7 +129,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertTrue($output);
     }
 
-    public function test_is_submitted() {
+    public function test_is_submitted(): void {
         $this->resetAfterTest();
 
         // This should assert to false as no record has been inserted.
@@ -149,7 +149,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertTrue($output);
     }
 
-    public function test_is_analyzed() {
+    public function test_is_analyzed(): void {
         $this->resetAfterTest();
 
         // This should assert to false as no record has been inserted.
@@ -170,14 +170,14 @@ class scheduler_test extends \advanced_testcase {
     }
 
     // Can't test because it's a protected function.
-    public function test_standard_search_params() {
+    public function test_standard_search_params(): void {
     }
 
     // Can't test because it's a protected function.
-    public function test_get_contextid() {
+    public function test_get_contextid(): void {
     }
 
-    public function test_get_datarecord() {
+    public function test_get_datarecord(): void {
         $this->resetAfterTest();
 
         $object = new scheduler();
@@ -200,11 +200,11 @@ class scheduler_test extends \advanced_testcase {
     }
 
     // No return statement.
-    public function test_process_scheduled_requests() {
+    public function test_process_scheduled_requests(): void {
 
     }
 
-    public function test_initialize_schedule() {
+    public function test_initialize_schedule(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -219,14 +219,14 @@ class scheduler_test extends \advanced_testcase {
         $this->assertEquals($record->contextlevel, 20);
     }
 
-    public function test_request_course_analysis() {
+    public function test_request_course_analysis(): void {
         $this->resetAfterTest();
 
         $output = scheduler::request_course_analysis(1);
         $this->assertTrue($output);
     }
 
-    public function test_create_course_schedule() {
+    public function test_create_course_schedule(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -237,7 +237,7 @@ class scheduler_test extends \advanced_testcase {
 
     }
 
-    public function test_delete_course_schedule() {
+    public function test_delete_course_schedule(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -251,7 +251,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertFalse($record);
     }
 
-    public function test_is_course_in_schedule() {
+    public function test_is_course_in_schedule(): void {
         $this->resetAfterTest();
 
         // This should assert to false as no record has been inserted.
@@ -264,7 +264,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertTrue($output);
     }
 
-    public function test_is_course_scheduled() {
+    public function test_is_course_scheduled(): void {
         $this->resetAfterTest();
 
         // This should assert to false as no record has been inserted.
@@ -283,7 +283,7 @@ class scheduler_test extends \advanced_testcase {
         $this->assertTrue($output);
     }
 
-    public function test_is_course_analyzed() {
+    public function test_is_course_analyzed(): void {
         $this->resetAfterTest();
         $object = new scheduler(10, 1);
 

admin/tool/brickfield/tests/tool_test.php

@@ -58,7 +58,7 @@ w==" alt="This is a bus." />';
 EOF;
 
 
-    public function test_build_all_accessibilitytools() {
+    public function test_build_all_accessibilitytools(): void {
         $tools = tool::build_all_accessibilitytools();
 
         $this->assertEquals($tools['errors']::toolshortname(), 'Error list');
@@ -68,14 +68,14 @@ EOF;
         $this->assertEquals($tools['advanced']::toolshortname(), 'Advanced');
     }
 
-    public function test_data_is_valid() {
+    public function test_data_is_valid(): void {
         $object = $this->getMockForAbstractClass(tool::class);
         $object->set_filter(new filter());
         $output = $object->data_is_valid();
         $this->assertFalse($output);
     }
 
-    public function test_can_access() {
+    public function test_can_access(): void {
         $this->resetAfterTest();
         $category = $this->getDataGenerator()->create_category();
         $filter = new filter(1, $category->id, 'tab', 3, 4);
@@ -86,14 +86,14 @@ EOF;
         $this->assertFalse($output);
     }
 
-    public function test_get_error_message() {
+    public function test_get_error_message(): void {
         $tool = $this->getMockForAbstractClass(tool::class);
 
         $output = $tool->get_error_message();
         $this->assertEquals($output, '');
     }
 
-    public function test_get_module_label() {
+    public function test_get_module_label(): void {
         $output = tool::get_module_label('core_course');
         $this->assertEquals($output, 'Course');
 
@@ -101,7 +101,7 @@ EOF;
         $this->assertEquals($output, 'Book');
     }
 
-    public function test_toplevel_arguments() {
+    public function test_toplevel_arguments(): void {
         $this->resetAfterTest();
         $category = $this->getDataGenerator()->create_category();
         $filter = new filter(1, $category->id, 'tab', 3, 4);
@@ -154,14 +154,14 @@ EOF;
      * @param string $content
      * @param bool $expectation
      */
-    public function test_base64_img_detected(string $content, bool $expectation) {
+    public function test_base64_img_detected(string $content, bool $expectation): void {
         $this->assertEquals(
             $expectation,
             tool::base64_img_detected($content)
         );
     }
 
-    public function test_truncate_base64() {
+    public function test_truncate_base64(): void {
         $truncated = tool::truncate_base64($this->base64img);
         $this->assertStringContainsString('<img src="data:image/gif;base64..."', $truncated);
     }

admin/tool/capability/tests/event/events_test.php

@@ -44,7 +44,7 @@ class events_test extends \advanced_testcase {
     /**
      * Test the report viewed event.
      */
-    public function test_report_viewed() {
+    public function test_report_viewed(): void {
         $event = \tool_capability\event\report_viewed::create();
 
         // Trigger and capture the event.

admin/tool/capability/tests/locallib_test.php

@@ -36,7 +36,7 @@ class locallib_test extends \advanced_testcase {
     /**
      * Test the function that gets the data - simple case.
      */
-    public function test_tool_capability_calculate_role_data() {
+    public function test_tool_capability_calculate_role_data(): void {
         global $DB;
 
         $data = tool_capability_calculate_role_data('mod/quiz:attempt', get_all_roles());
@@ -58,7 +58,7 @@ class locallib_test extends \advanced_testcase {
     /**
      * Test the function that gets the data - simple case.
      */
-    public function test_tool_capability_calculate_role_data_orphan_contexts() {
+    public function test_tool_capability_calculate_role_data_orphan_contexts(): void {
         global $DB;
         $this->resetAfterTest();
 

admin/tool/cohortroles/db/upgrade.php

@@ -48,5 +48,8 @@ function xmldb_tool_cohortroles_upgrade($oldversion) {
     // Automatically generated Moodle v4.3.0 release upgrade line.
     // Put any upgrade step following this.
 
+    // Automatically generated Moodle v4.4.0 release upgrade line.
+    // Put any upgrade step following this.
+
     return true;
 }

admin/tool/cohortroles/tests/api_test.php

@@ -53,7 +53,7 @@ class api_test extends \advanced_testcase {
         cohort_add_member($this->cohort->id, $this->userassignover->id);
     }
 
-    public function test_create_cohort_role_assignment_without_permission() {
+    public function test_create_cohort_role_assignment_without_permission(): void {
         $this->setUser($this->userassignto);
         $params = (object) array(
             'userid' => $this->userassignto->id,
@@ -64,7 +64,7 @@ class api_test extends \advanced_testcase {
         api::create_cohort_role_assignment($params);
     }
 
-    public function test_create_cohort_role_assignment_with_invalid_data() {
+    public function test_create_cohort_role_assignment_with_invalid_data(): void {
         $this->setAdminUser();
         $params = (object) array(
             'userid' => $this->userassignto->id,
@@ -75,7 +75,7 @@ class api_test extends \advanced_testcase {
         api::create_cohort_role_assignment($params);
     }
 
-    public function test_create_cohort_role_assignment() {
+    public function test_create_cohort_role_assignment(): void {
         $this->setAdminUser();
         $params = (object) array(
             'userid' => $this->userassignto->id,
@@ -89,7 +89,7 @@ class api_test extends \advanced_testcase {
         $this->assertEquals($result->get('cohortid'), $this->cohort->id);
     }
 
-    public function test_delete_cohort_role_assignment_without_permission() {
+    public function test_delete_cohort_role_assignment_without_permission(): void {
         $this->setAdminUser();
         $params = (object) array(
             'userid' => $this->userassignto->id,
@@ -102,7 +102,7 @@ class api_test extends \advanced_testcase {
         api::delete_cohort_role_assignment($result->get('id'));
     }
 
-    public function test_delete_cohort_role_assignment_with_invalid_data() {
+    public function test_delete_cohort_role_assignment_with_invalid_data(): void {
         $this->setAdminUser();
         $params = (object) array(
             'userid' => $this->userassignto->id,
@@ -114,7 +114,7 @@ class api_test extends \advanced_testcase {
         api::delete_cohort_role_assignment($result->get('id') + 1);
     }
 
-    public function test_delete_cohort_role_assignment() {
+    public function test_delete_cohort_role_assignment(): void {
         $this->setAdminUser();
         // Create a cohort role assigment.
         $params = (object) [
@@ -151,7 +151,7 @@ class api_test extends \advanced_testcase {
     /**
      * 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() {
+    public function test_delete_cohort_role_assignment_cohorts_having_same_members(): void {
         $this->setAdminUser();
 
         // Create 2 cohorts, with a 1 user (user1) present in both,
@@ -203,7 +203,7 @@ class api_test extends \advanced_testcase {
         $this->assertContains($user2->id, $removedusers);
     }
 
-    public function test_list_cohort_role_assignments() {
+    public function test_list_cohort_role_assignments(): void {
         $this->setAdminUser();
         $params = (object) array(
             'userid' => $this->userassignto->id,
@@ -217,7 +217,7 @@ class api_test extends \advanced_testcase {
         $this->assertEquals($list[0], $result);
     }
 
-    public function test_count_cohort_role_assignments() {
+    public function test_count_cohort_role_assignments(): void {
         $this->setAdminUser();
         $params = (object) array(
             'userid' => $this->userassignto->id,
@@ -230,7 +230,7 @@ class api_test extends \advanced_testcase {
         $this->assertEquals($count, 1);
     }
 
-    public function test_sync_all_cohort_roles() {
+    public function test_sync_all_cohort_roles(): void {
         $this->setAdminUser();
         $params = (object) array(
             'userid' => $this->userassignto->id,

admin/tool/cohortroles/tests/privacy/provider_test.php

@@ -51,7 +51,7 @@ class provider_test extends \core_privacy\tests\provider_testcase {
     /**
      * Test for provider::get_contexts_for_userid().
      */
-    public function test_get_contexts_for_userid() {
+    public function test_get_contexts_for_userid(): void {
         global $DB;
 
         // Test setup.
@@ -94,7 +94,7 @@ class provider_test extends \core_privacy\tests\provider_testcase {
     /**
      * Test for provider::export_user_data().
      */
-    public function test_export_user_data() {
+    public function test_export_user_data(): void {
         // Test setup.
         $user = $this->getDataGenerator()->create_user();
         $this->setUser($user);
@@ -144,7 +144,7 @@ class provider_test extends \core_privacy\tests\provider_testcase {
     /**
      * Test for provider::delete_data_for_all_users_in_context().
      */
-    public function test_delete_data_for_all_users_in_context() {
+    public function test_delete_data_for_all_users_in_context(): void {
         global $DB;
 
         // Test setup.
@@ -203,7 +203,7 @@ class provider_test extends \core_privacy\tests\provider_testcase {
     /**
      * Test for provider::delete_data_for_user().
      */
-    public function test_delete_data_for_user() {
+    public function test_delete_data_for_user(): void {
         global $DB;
 
         // Test setup.
@@ -251,7 +251,7 @@ class provider_test extends \core_privacy\tests\provider_testcase {
     /**
      * Test that only users within a course context are fetched.
      */
-    public function test_get_users_in_context() {
+    public function test_get_users_in_context(): void {
         $component = 'tool_cohortroles';
 
         // Create a user.
@@ -293,7 +293,7 @@ class provider_test extends \core_privacy\tests\provider_testcase {
     /**
      * Test that data for users in approved userlist is deleted.
      */
-    public function test_delete_data_for_users() {
+    public function test_delete_data_for_users(): void {
         $component = 'tool_cohortroles';
 
         // Create user1.

admin/tool/customlang/db/upgrade.php

@@ -33,5 +33,8 @@ function xmldb_tool_customlang_upgrade($oldversion) {
     // Automatically generated Moodle v4.3.0 release upgrade line.
     // Put any upgrade step following this.
 
+    // Automatically generated Moodle v4.4.0 release upgrade line.
+    // Put any upgrade step following this.
+
     return true;
 }

admin/tool/dataprivacy/classes/data_request.php

@@ -156,9 +156,9 @@ class data_request extends persistent {
                 case api::DATAREQUEST_STATUS_EXPIRED:
                     $result = true;
                     break;
-                // Complete requests are expired if the expiry time has elapsed.
+                // Complete requests are expired if the expiry time is a positive value, and has elapsed.
                 case api::DATAREQUEST_STATUS_DOWNLOAD_READY:
-                    $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry');
+                    $expiryseconds = (int) get_config('tool_dataprivacy', 'privacyrequestexpiry');
                     if ($expiryseconds > 0 && time() >= ($request->get('timemodified') + $expiryseconds)) {
                         $result = true;
                     }
@@ -179,7 +179,12 @@ class data_request extends persistent {
     public static function get_expired_requests($userid = 0) {
         global $DB;
 
-        $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry');
+        // Complete requests are expired if the expiry time is a positive value, and has elapsed.
+        $expiryseconds = (int) get_config('tool_dataprivacy', 'privacyrequestexpiry');
+        if ($expiryseconds <= 0) {
+            return [];
+        }
+
         $expirytime = strtotime("-{$expiryseconds} second");
         $table = self::TABLE;
         $sqlwhere = 'type = :export_type AND status = :completestatus AND timemodified <= :expirytime';

admin/tool/dataprivacy/classes/hook_callbacks.php

@@ -45,6 +45,7 @@ class hook_callbacks {
             $hook->add_html(
                 html_writer::div(
                     html_writer::link($url, get_string('dataretentionsummary', 'tool_dataprivacy')),
+                    'tool_dataprivacy',
                 ),
             );
         }

admin/tool/dataprivacy/db/upgrade.php

@@ -103,5 +103,8 @@ function xmldb_tool_dataprivacy_upgrade($oldversion) {
     // Automatically generated Moodle v4.3.0 release upgrade line.
     // Put any upgrade step following this.
 
+    // Automatically generated Moodle v4.4.0 release upgrade line.
+    // Put any upgrade step following this.
+
     return true;
 }

admin/tool/dataprivacy/tests/api_test.php

@@ -37,7 +37,7 @@ class api_test extends \advanced_testcase {
      * Ensure that the check_can_manage_data_registry function fails cap testing when a user without capabilities is
      * tested with the default context.
      */
-    public function test_check_can_manage_data_registry_admin() {
+    public function test_check_can_manage_data_registry_admin(): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -49,7 +49,7 @@ class api_test extends \advanced_testcase {
      * Ensure that the check_can_manage_data_registry function fails cap testing when a user without capabilities is
      * tested with the default context.
      */
-    public function test_check_can_manage_data_registry_without_cap_default() {
+    public function test_check_can_manage_data_registry_without_cap_default(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user();
@@ -63,7 +63,7 @@ class api_test extends \advanced_testcase {
      * Ensure that the check_can_manage_data_registry function fails cap testing when a user without capabilities is
      * tested with the default context.
      */
-    public function test_check_can_manage_data_registry_without_cap_system() {
+    public function test_check_can_manage_data_registry_without_cap_system(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user();
@@ -77,7 +77,7 @@ class api_test extends \advanced_testcase {
      * Ensure that the check_can_manage_data_registry function fails cap testing when a user without capabilities is
      * tested with the default context.
      */
-    public function test_check_can_manage_data_registry_without_cap_own_user() {
+    public function test_check_can_manage_data_registry_without_cap_own_user(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user();
@@ -90,7 +90,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::update_request_status().
      */
-    public function test_update_request_status() {
+    public function test_update_request_status(): void {
         $this->resetAfterTest();
 
         $generator = new testing_data_generator();
@@ -138,7 +138,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::get_site_dpos() when there are no users with the DPO role.
      */
-    public function test_get_site_dpos_no_dpos() {
+    public function test_get_site_dpos_no_dpos(): void {
         $this->resetAfterTest();
 
         $admin = get_admin();
@@ -152,7 +152,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::get_site_dpos() when there are no users with the DPO role.
      */
-    public function test_get_site_dpos() {
+    public function test_get_site_dpos(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -189,7 +189,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for \tool_dataprivacy\api::get_assigned_privacy_officer_roles().
      */
-    public function test_get_assigned_privacy_officer_roles() {
+    public function test_get_assigned_privacy_officer_roles(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -232,7 +232,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::approve_data_request().
      */
-    public function test_approve_data_request() {
+    public function test_approve_data_request(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -276,7 +276,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::approve_data_request() when allow filtering of exports by course.
      */
-    public function test_approve_data_request_with_allow_filtering() {
+    public function test_approve_data_request_with_allow_filtering(): void {
         global $DB;
         $this->resetAfterTest();
         set_config('allowfiltering', 1, 'tool_dataprivacy');
@@ -325,7 +325,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::approve_data_request() when called by a user who doesn't have the DPO role.
      */
-    public function test_approve_data_request_non_dpo_user() {
+    public function test_approve_data_request_non_dpo_user(): void {
         $this->resetAfterTest();
 
         $generator = new testing_data_generator();
@@ -347,7 +347,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::add_request_contexts_with_status().
      */
-    public function test_add_request_contexts_with_status() {
+    public function test_add_request_contexts_with_status(): void {
         global $DB;
         $this->resetAfterTest();
         set_config('allowfiltering', 1, 'tool_dataprivacy');
@@ -385,7 +385,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test that deletion requests for the primary admin are rejected
      */
-    public function test_reject_data_deletion_request_primary_admin() {
+    public function test_reject_data_deletion_request_primary_admin(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -408,7 +408,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::can_contact_dpo()
      */
-    public function test_can_contact_dpo() {
+    public function test_can_contact_dpo(): void {
         $this->resetAfterTest();
 
         // Default ('contactdataprotectionofficer' is disabled by default).
@@ -426,7 +426,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::can_manage_data_requests()
      */
-    public function test_can_manage_data_requests() {
+    public function test_can_manage_data_requests(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -471,7 +471,7 @@ class api_test extends \advanced_testcase {
      * Test that a user who has no capability to make any data requests for children cannot create data requests for any
      * other user.
      */
-    public function test_can_create_data_request_for_user_no() {
+    public function test_can_create_data_request_for_user_no(): void {
         $this->resetAfterTest();
 
         $parent = $this->getDataGenerator()->create_user();
@@ -485,7 +485,7 @@ class api_test extends \advanced_testcase {
      * Test that a user who has the capability to make any data requests for one other user cannot create data requests
      * for any other user.
      */
-    public function test_can_create_data_request_for_user_some() {
+    public function test_can_create_data_request_for_user_some(): void {
         $this->resetAfterTest();
 
         $parent = $this->getDataGenerator()->create_user();
@@ -505,7 +505,7 @@ class api_test extends \advanced_testcase {
      * Test that a user who has the capability to make any data requests for one other user cannot create data requests
      * for any other user.
      */
-    public function test_can_create_data_request_for_user_own_child() {
+    public function test_can_create_data_request_for_user_own_child(): void {
         $this->resetAfterTest();
 
         $parent = $this->getDataGenerator()->create_user();
@@ -524,7 +524,7 @@ class api_test extends \advanced_testcase {
      * Test that a user who has no capability to make any data requests for children cannot create data requests for any
      * other user.
      */
-    public function test_require_can_create_data_request_for_user_no() {
+    public function test_require_can_create_data_request_for_user_no(): void {
         $this->resetAfterTest();
 
         $parent = $this->getDataGenerator()->create_user();
@@ -539,7 +539,7 @@ class api_test extends \advanced_testcase {
      * Test that a user who has the capability to make any data requests for one other user cannot create data requests
      * for any other user.
      */
-    public function test_require_can_create_data_request_for_user_some() {
+    public function test_require_can_create_data_request_for_user_some(): void {
         $this->resetAfterTest();
 
         $parent = $this->getDataGenerator()->create_user();
@@ -560,7 +560,7 @@ class api_test extends \advanced_testcase {
      * Test that a user who has the capability to make any data requests for one other user cannot create data requests
      * for any other user.
      */
-    public function test_require_can_create_data_request_for_user_own_child() {
+    public function test_require_can_create_data_request_for_user_own_child(): void {
         $this->resetAfterTest();
 
         $parent = $this->getDataGenerator()->create_user();
@@ -578,7 +578,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::can_download_data_request_for_user()
      */
-    public function test_can_download_data_request_for_user() {
+    public function test_can_download_data_request_for_user(): void {
         $this->resetAfterTest();
 
         $generator = $this->getDataGenerator();
@@ -700,7 +700,7 @@ class api_test extends \advanced_testcase {
         $expectedstatus,
         $expectedtaskcount,
         $allowfiltering,
-    ) {
+    ): void {
         global $USER;
 
         $this->resetAfterTest();
@@ -751,7 +751,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::create_data_request() made by a parent.
      */
-    public function test_create_data_request_by_parent() {
+    public function test_create_data_request_by_parent(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -786,7 +786,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::deny_data_request()
      */
-    public function test_deny_data_request() {
+    public function test_deny_data_request(): void {
         $this->resetAfterTest();
 
         $generator = new testing_data_generator();
@@ -841,7 +841,7 @@ class api_test extends \advanced_testcase {
      * @param boolean $fetchall Whether to fetch all records.
      * @param int[] $statuses Status filters.
      */
-    public function test_get_data_requests($usertype, $fetchall, $statuses) {
+    public function test_get_data_requests($usertype, $fetchall, $statuses): void {
         $this->resetAfterTest();
 
         $generator = new testing_data_generator();
@@ -952,7 +952,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::get_approved_contextlist_collection_for_request.
      */
-    public function test_get_approved_contextlist_collection_for_request() {
+    public function test_get_approved_contextlist_collection_for_request(): void {
         $this->resetAfterTest();
         set_config('allowfiltering', 1, 'tool_dataprivacy');
         $this->setAdminUser();
@@ -1021,7 +1021,7 @@ class api_test extends \advanced_testcase {
      * @param int $status The request status.
      * @param bool $expected The expected result.
      */
-    public function test_has_ongoing_request($status, $expected) {
+    public function test_has_ongoing_request($status, $expected): void {
         $this->resetAfterTest();
 
         $generator = new testing_data_generator();
@@ -1045,7 +1045,7 @@ class api_test extends \advanced_testcase {
      * @param int $status The request status
      * @param bool $expected The expected result
      */
-    public function test_is_active($status, $expected) {
+    public function test_is_active($status, $expected): void {
         // Check if this request is ongoing.
         $result = api::is_active($status);
         $this->assertEquals($expected, $result);
@@ -1054,7 +1054,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test for api::is_site_dpo()
      */
-    public function test_is_site_dpo() {
+    public function test_is_site_dpo(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -1108,7 +1108,7 @@ class api_test extends \advanced_testcase {
      * @param string $typestringid The request lang string identifier
      * @param string $comments The requestor's message to the DPO.
      */
-    public function test_notify_dpo($byadmin, $type, $typestringid, $comments) {
+    public function test_notify_dpo($byadmin, $type, $typestringid, $comments): void {
         $this->resetAfterTest();
 
         $generator = new testing_data_generator();
@@ -1150,7 +1150,7 @@ class api_test extends \advanced_testcase {
      *
      * @return null
      */
-    public function test_purpose_crud() {
+    public function test_purpose_crud(): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -1192,7 +1192,7 @@ class api_test extends \advanced_testcase {
      *
      * @return null
      */
-    public function test_category_crud() {
+    public function test_category_crud(): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -1230,7 +1230,7 @@ class api_test extends \advanced_testcase {
      *
      * @return null
      */
-    public function test_context_instances() {
+    public function test_context_instances(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -1266,7 +1266,7 @@ class api_test extends \advanced_testcase {
      *
      * @return null
      */
-    public function test_contextlevel() {
+    public function test_contextlevel(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -1302,7 +1302,7 @@ class api_test extends \advanced_testcase {
      *
      * @return null
      */
-    public function test_effective_contextlevel_defaults() {
+    public function test_effective_contextlevel_defaults(): void {
         $this->setAdminUser();
 
         $this->resetAfterTest();
@@ -1340,7 +1340,7 @@ class api_test extends \advanced_testcase {
     /**
      * Ensure that when nothing is configured, all values return false.
      */
-    public function test_get_effective_contextlevel_unset() {
+    public function test_get_effective_contextlevel_unset(): void {
         // Before setup, get_effective_contextlevel_purpose will return false.
         $this->assertFalse(api::get_effective_contextlevel_category(CONTEXT_SYSTEM));
         $this->assertFalse(api::get_effective_contextlevel_purpose(CONTEXT_SYSTEM));
@@ -1352,7 +1352,7 @@ class api_test extends \advanced_testcase {
     /**
      * Ensure that when nothing is configured, all values return false.
      */
-    public function test_get_effective_context_unset() {
+    public function test_get_effective_context_unset(): void {
         // Before setup, get_effective_contextlevel_purpose will return false.
         $this->assertFalse(api::get_effective_context_category(\context_system::instance()));
         $this->assertFalse(api::get_effective_context_purpose(\context_system::instance()));
@@ -1364,7 +1364,7 @@ class api_test extends \advanced_testcase {
      * @dataProvider invalid_effective_contextlevel_provider
      * @param   int $contextlevel
      */
-    public function test_set_contextlevel_invalid_contextlevels($contextlevel) {
+    public function test_set_contextlevel_invalid_contextlevels($contextlevel): void {
 
         $this->expectException(\coding_exception::class);
         api::set_contextlevel((object) [
@@ -1376,7 +1376,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test effective contextlevel return.
      */
-    public function test_effective_contextlevel() {
+    public function test_effective_contextlevel(): void {
         $this->resetAfterTest();
 
         // Set the initial purpose and category.
@@ -1424,7 +1424,7 @@ class api_test extends \advanced_testcase {
      * @dataProvider invalid_effective_contextlevel_provider
      * @param   int $contextlevel
      */
-    public function test_effective_contextlevel_invalid_contextlevels($contextlevel) {
+    public function test_effective_contextlevel_invalid_contextlevels($contextlevel): void {
         $this->resetAfterTest();
 
         $purpose1 = api::create_purpose((object)['name' => 'p1', 'retentionperiod' => 'PT1H', 'lawfulbases' => 'gdpr_art_6_1_a']);
@@ -1454,7 +1454,7 @@ class api_test extends \advanced_testcase {
     /**
      * Ensure that context inheritance works up the context tree.
      */
-    public function test_effective_context_inheritance() {
+    public function test_effective_context_inheritance(): void {
         $this->resetAfterTest();
 
         $systemdata = $this->create_and_set_purpose_for_contextlevel('PT1S', CONTEXT_SYSTEM);
@@ -1734,7 +1734,7 @@ class api_test extends \advanced_testcase {
      * Although it should not be possible to set hard INHERIT values at this level, there may be legacy data which still
      * contains this.
      */
-    public function test_effective_context_inheritance_explicitly_set() {
+    public function test_effective_context_inheritance_explicitly_set(): void {
         $this->resetAfterTest();
 
         $systemdata = $this->create_and_set_purpose_for_contextlevel('PT1S', CONTEXT_SYSTEM);
@@ -1861,7 +1861,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test that delete requests do not filter out protected purpose contexts if the the site is properly configured.
      */
-    public function test_get_approved_contextlist_collection_for_collection_delete_course_no_site_config() {
+    public function test_get_approved_contextlist_collection_for_collection_delete_course_no_site_config(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user();
@@ -1903,7 +1903,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test that delete requests do not filter out protected purpose contexts if they are already expired.
      */
-    public function test_get_approved_contextlist_collection_for_collection_delete_course_expired_protected() {
+    public function test_get_approved_contextlist_collection_for_collection_delete_course_expired_protected(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -1936,7 +1936,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test that delete requests does filter out protected purpose contexts which are not expired.
      */
-    public function test_get_approved_contextlist_collection_for_collection_delete_course_unexpired_protected() {
+    public function test_get_approved_contextlist_collection_for_collection_delete_course_unexpired_protected(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1Y');
@@ -1969,7 +1969,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test that delete requests do not filter out unexpired contexts if they are not protected.
      */
-    public function test_get_approved_contextlist_collection_for_collection_delete_course_unexpired_unprotected() {
+    public function test_get_approved_contextlist_collection_for_collection_delete_course_unexpired_unprotected(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1Y');
@@ -2040,7 +2040,7 @@ class api_test extends \advanced_testcase {
      * @param bool $foractivity Whether to set defaults for an activity.
      * @param bool $override Whether to override instances.
      */
-    public function test_set_context_defaults($contextlevel, $inheritcategory, $inheritpurpose, $foractivity, $override) {
+    public function test_set_context_defaults($contextlevel, $inheritcategory, $inheritpurpose, $foractivity, $override): void {
         $this->resetAfterTest();
 
         $generator = $this->getDataGenerator();
@@ -2224,7 +2224,7 @@ class api_test extends \advanced_testcase {
     /**
      * Ensure that the find_ongoing_request_types_for_users only returns requests which are active.
      */
-    public function test_find_ongoing_request_types_for_users() {
+    public function test_find_ongoing_request_types_for_users(): void {
         $this->resetAfterTest();
 
         // Create users and their requests:.
@@ -2360,7 +2360,7 @@ class api_test extends \advanced_testcase {
      *
      * @throws coding_exception
      */
-    public function test_can_create_data_deletion_request_for_self_no() {
+    public function test_can_create_data_deletion_request_for_self_no(): void {
         $this->resetAfterTest();
         $userid = $this->getDataGenerator()->create_user()->id;
         $roleid = $this->getDataGenerator()->create_role();
@@ -2373,7 +2373,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test primary admin cannot create data deletion request for themselves
      */
-    public function test_can_create_data_deletion_request_for_self_primary_admin() {
+    public function test_can_create_data_deletion_request_for_self_primary_admin(): void {
         $this->resetAfterTest();
         $this->setAdminUser();
         $this->assertFalse(api::can_create_data_deletion_request_for_self());
@@ -2382,7 +2382,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test secondary admin can create data deletion request for themselves
      */
-    public function test_can_create_data_deletion_request_for_self_secondary_admin() {
+    public function test_can_create_data_deletion_request_for_self_secondary_admin(): void {
         $this->resetAfterTest();
 
         $admin1 = $this->getDataGenerator()->create_user();
@@ -2403,7 +2403,7 @@ class api_test extends \advanced_testcase {
      *
      * @throws coding_exception
      */
-    public function test_can_create_data_deletion_request_for_self_yes() {
+    public function test_can_create_data_deletion_request_for_self_yes(): void {
         $this->resetAfterTest();
         $userid = $this->getDataGenerator()->create_user()->id;
         $this->setUser($userid);
@@ -2417,7 +2417,7 @@ class api_test extends \advanced_testcase {
      * @throws coding_exception
      * @throws dml_exception
      */
-    public function test_can_create_data_deletion_request_for_other_no() {
+    public function test_can_create_data_deletion_request_for_other_no(): void {
         $this->resetAfterTest();
         $userid = $this->getDataGenerator()->create_user()->id;
         $this->setUser($userid);
@@ -2430,7 +2430,7 @@ class api_test extends \advanced_testcase {
      *
      * @throws coding_exception
      */
-    public function test_can_create_data_deletion_request_for_other_yes() {
+    public function test_can_create_data_deletion_request_for_other_yes(): void {
         $this->resetAfterTest();
         $userid = $this->getDataGenerator()->create_user()->id;
         $roleid = $this->getDataGenerator()->create_role();
@@ -2448,7 +2448,7 @@ class api_test extends \advanced_testcase {
      * @throws coding_exception
      * @throws dml_exception
      */
-    public function test_can_create_data_deletion_request_for_children() {
+    public function test_can_create_data_deletion_request_for_children(): void {
         $this->resetAfterTest();
 
         $parent = $this->getDataGenerator()->create_user();
@@ -2490,7 +2490,7 @@ class api_test extends \advanced_testcase {
      * @dataProvider queue_data_request_task_provider
      * @param bool $withuserid
      */
-    public function test_queue_data_request_task(bool $withuserid) {
+    public function test_queue_data_request_task(bool $withuserid): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -2546,7 +2546,7 @@ class api_test extends \advanced_testcase {
      * @param bool $value The setting's value.
      * @param bool $expected The expected result.
      */
-    public function test_is_automatic_request_approval_on($setting, $type, $value, $expected) {
+    public function test_is_automatic_request_approval_on($setting, $type, $value, $expected): void {
         $this->resetAfterTest();
 
         if ($value !== null) {
@@ -2713,7 +2713,7 @@ class api_test extends \advanced_testcase {
     /**
      * Test api validate_create_data_request.
      */
-    public function test_validate_create_data_request() {
+    public function test_validate_create_data_request(): void {
         $this->resetAfterTest();
 
         $systemcontext = \context_system::instance();

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

@@ -54,6 +54,7 @@ Feature: Data delete from the privacy API
     And I should see "Awaiting approval" in the "Victim User 1" "table_row"
     And I open the action menu in "Victim User 1" "table_row"
     And I follow "Approve request"
+    And I wait until "Approve request" "button" exists
     And I press "Approve request"
     And I should see "Approved" in the "Victim User 1" "table_row"
     And I run all adhoc tasks
@@ -80,6 +81,7 @@ Feature: Data delete from the privacy API
     And I navigate to "Users > Privacy and policies > Data requests" in site administration
     And I open the action menu in "Victim User 1" "table_row"
     And I follow "Approve request"
+    And I wait until "Approve request" "button" exists
     And I press "Approve request"
 
     And I log out
@@ -115,6 +117,7 @@ Feature: Data delete from the privacy API
     And I navigate to "Users > Privacy and policies > Data requests" in site administration
     And I open the action menu in "Victim User 1" "table_row"
     And I follow "Approve request"
+    And I wait until "Approve request" "button" exists
     And I press "Approve request"
 
     And I log out

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

@@ -48,13 +48,15 @@ Feature: Data export from the privacy API
     And I should see "Awaiting approval" in the "Victim User 1" "table_row"
     And I open the action menu in "Victim User 1" "table_row"
     And I follow "Approve request"
+    And I wait until "Approve request" "button" exists
     And I press "Approve request"
     And I should see "Approved" in the "Victim User 1" "table_row"
     And I run all adhoc tasks
     And I reload the page
     And I should see "Download ready" in the "Victim User 1" "table_row"
     And I open the action menu in "Victim User 1" "table_row"
-    And following "Download" should download between "1" and "200000" bytes
+    And following "Download" should download a file that:
+      | Contains file in zip | index.html |
     And the following config values are set as admin:
       | privacyrequestexpiry | 1 | tool_dataprivacy |
     And I wait "1" seconds
@@ -89,7 +91,8 @@ Feature: Data export from the privacy API
     And I reload the page
     And I should see "Download ready" in the "Export all of my personal data" "table_row"
     And I open the action menu in "Victim User 1" "table_row"
-    And following "Download" should download between "1" and "200000" bytes
+    And following "Download" should download a file that:
+      | Contains file in zip | index.html |
 
     And the following config values are set as admin:
       | privacyrequestexpiry | 1 | tool_dataprivacy |
@@ -115,6 +118,7 @@ Feature: Data export from the privacy API
     And I navigate to "Users > Privacy and policies > Data requests" in site administration
     And I open the action menu in "Victim User 1" "table_row"
     And I follow "Approve request"
+    And I wait until "Approve request" "button" exists
     And I press "Approve request"
 
     And I log out
@@ -126,7 +130,8 @@ Feature: Data export from the privacy API
     And I reload the page
     And I should see "Download ready" in the "Victim User 1" "table_row"
     And I open the action menu in "Victim User 1" "table_row"
-    And following "Download" should download between "1" and "200000" bytes
+    And following "Download" should download a file that:
+      | Contains file in zip | index.html |
 
     And the following config values are set as admin:
       | privacyrequestexpiry | 1 | tool_dataprivacy |
@@ -185,7 +190,8 @@ Feature: Data export from the privacy API
     And I reload the page
     And I should see "Download ready" in the "Victim User 1" "table_row"
     And I open the action menu in "Victim User 1" "table_row"
-    And following "Download" should download between "1" and "170000" bytes
+    And following "Download" should download a file that:
+      | Contains file in zip | index.html |
     And the following config values are set as admin:
       | privacyrequestexpiry | 1 | tool_dataprivacy |
     And I wait "1" seconds
@@ -229,7 +235,8 @@ Feature: Data export from the privacy API
     And I reload the page
     And I should see "Download ready" in the "Victim User 1" "table_row"
     And I open the action menu in "Victim User 1" "table_row"
-    And following "Download" should download between "1" and "180000" bytes
+    And following "Download" should download a file that:
+      | Contains file in zip | index.html |
 
   @javascript
   Scenario: Filter before export data for a user and download it in the view request action
@@ -262,4 +269,5 @@ Feature: Data export from the privacy API
     And I reload the page
     And I should see "Download ready" in the "Victim User 1" "table_row"
     And I open the action menu in "Victim User 1" "table_row"
-    And following "Download" should download between "1" and "180000" bytes
+    And following "Download" should download a file that:
+      | Contains file in zip | index.html |

admin/tool/dataprivacy/tests/data_registry_test.php

@@ -30,7 +30,7 @@ class data_registry_test extends \advanced_testcase {
      *
      * This test is not great because we only test a limited set of values. This is a fault of the underlying API.
      */
-    public function test_get_effective_context_value_invalid_element() {
+    public function test_get_effective_context_value_invalid_element(): void {
         $this->expectException(\coding_exception::class);
         data_registry::get_effective_context_value(\context_system::instance(), 'invalid');
     }
@@ -40,7 +40,7 @@ class data_registry_test extends \advanced_testcase {
      *
      * This test is not great because we only test a limited set of values. This is a fault of the underlying API.
      */
-    public function test_get_effective_contextlevel_value_invalid_element() {
+    public function test_get_effective_contextlevel_value_invalid_element(): void {
         $this->expectException(\coding_exception::class);
         data_registry::get_effective_contextlevel_value(\context_system::instance(), 'invalid');
     }

admin/tool/dataprivacy/tests/data_request_test.php

@@ -93,7 +93,7 @@ class data_request_test extends data_privacy_testcase {
      * @param       bool    $resettable
      * @param       bool    $active
      */
-    public function test_pseudo_states_export(int $status, bool $resettable, bool $active) {
+    public function test_pseudo_states_export(int $status, bool $resettable, bool $active): void {
         $uut = new \tool_dataprivacy\data_request();
         $uut->set('status', $status);
         $uut->set('type', api::DATAREQUEST_TYPE_EXPORT);
@@ -110,7 +110,7 @@ class data_request_test extends data_privacy_testcase {
      * @param       bool    $resettable
      * @param       bool    $active
      */
-    public function test_pseudo_states_delete(int $status, bool $resettable, bool $active) {
+    public function test_pseudo_states_delete(int $status, bool $resettable, bool $active): void {
         $uut = new \tool_dataprivacy\data_request();
         $uut->set('status', $status);
         $uut->set('type', api::DATAREQUEST_TYPE_DELETE);
@@ -125,7 +125,7 @@ class data_request_test extends data_privacy_testcase {
      * @dataProvider        status_state_provider
      * @param       int     $status
      */
-    public function test_can_reset_others($status) {
+    public function test_can_reset_others($status): void {
         $uut = new \tool_dataprivacy\data_request();
         $uut->set('status', $status);
         $uut->set('type', api::DATAREQUEST_TYPE_OTHERS);
@@ -155,7 +155,7 @@ class data_request_test extends data_privacy_testcase {
      * @dataProvider        non_resettable_provider
      * @param       int     $status
      */
-    public function test_non_resubmit_request($status) {
+    public function test_non_resubmit_request($status): void {
         $uut = new \tool_dataprivacy\data_request();
         $uut->set('status', $status);
 
@@ -168,7 +168,7 @@ class data_request_test extends data_privacy_testcase {
     /**
      * Ensure that a rejected request can be reset.
      */
-    public function test_resubmit_request() {
+    public function test_resubmit_request(): void {
         $this->resetAfterTest();
 
         $uut = new \tool_dataprivacy\data_request();
@@ -192,7 +192,7 @@ class data_request_test extends data_privacy_testcase {
     /**
      * Ensure that an active request can be reset.
      */
-    public function test_resubmit_active_request() {
+    public function test_resubmit_active_request(): void {
         $this->resetAfterTest();
 
         $uut = new \tool_dataprivacy\data_request();

admin/tool/dataprivacy/tests/expired_contexts_test.php

@@ -90,7 +90,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with no lastaccess is not flagged for deletion.
      */
-    public function test_flag_not_setup() {
+    public function test_flag_not_setup(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user();
@@ -111,7 +111,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with no lastaccess is not flagged for deletion.
      */
-    public function test_flag_user_no_lastaccess() {
+    public function test_flag_user_no_lastaccess(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -134,7 +134,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a recent lastaccess is not flagged for deletion.
      */
-    public function test_flag_user_recent_lastaccess() {
+    public function test_flag_user_recent_lastaccess(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -157,7 +157,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a lastaccess in the past is flagged for deletion.
      */
-    public function test_flag_user_past_lastaccess() {
+    public function test_flag_user_past_lastaccess(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -181,7 +181,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a lastaccess in the past but active enrolments is not flagged for deletion.
      */
-    public function test_flag_user_past_lastaccess_still_enrolled() {
+    public function test_flag_user_past_lastaccess_still_enrolled(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -209,7 +209,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a lastaccess in the past and no active enrolments is flagged for deletion.
      */
-    public function test_flag_user_update_existing() {
+    public function test_flag_user_update_existing(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'P5Y');
@@ -241,7 +241,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a lastaccess in the past and expired enrolments.
      */
-    public function test_flag_user_past_lastaccess_unexpired_past_enrolment() {
+    public function test_flag_user_past_lastaccess_unexpired_past_enrolment(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'P1Y');
@@ -269,7 +269,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a lastaccess in the past and expired enrolments.
      */
-    public function test_flag_user_past_override_role() {
+    public function test_flag_user_past_override_role(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -303,7 +303,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a lastaccess in the past and expired enrolments.
      */
-    public function test_flag_user_past_lastaccess_expired_enrolled() {
+    public function test_flag_user_past_lastaccess_expired_enrolled(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -332,7 +332,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Ensure that a user with a lastaccess in the past and enrolments without a course end date are respected
      * correctly.
      */
-    public function test_flag_user_past_lastaccess_missing_enddate_required() {
+    public function test_flag_user_past_lastaccess_missing_enddate_required(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -364,7 +364,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Ensure that a user with a lastaccess in the past and enrolments without a course end date are respected
      * correctly when the end date is not required.
      */
-    public function test_flag_user_past_lastaccess_missing_enddate_not_required() {
+    public function test_flag_user_past_lastaccess_missing_enddate_not_required(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -395,7 +395,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a recent lastaccess is not flagged for deletion.
      */
-    public function test_flag_user_recent_lastaccess_existing_record() {
+    public function test_flag_user_recent_lastaccess_existing_record(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -429,7 +429,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a recent lastaccess is not flagged for deletion.
      */
-    public function test_flag_user_retention_changed() {
+    public function test_flag_user_retention_changed(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -469,7 +469,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a historically expired expired block record child is cleaned up.
      */
-    public function test_flag_user_historic_block_unapproved() {
+    public function test_flag_user_historic_block_unapproved(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -506,7 +506,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a block which has a default retention period which has not expired, is still expired.
      */
-    public function test_flag_user_historic_unexpired_child() {
+    public function test_flag_user_historic_unexpired_child(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -534,7 +534,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course with no end date is not flagged.
      */
-    public function test_flag_course_no_enddate() {
+    public function test_flag_course_no_enddate(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -553,7 +553,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course with an end date in the distant past, but a child which is unexpired is not flagged.
      */
-    public function test_flag_course_past_enddate_future_child() {
+    public function test_flag_course_past_enddate_future_child(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'P5Y');
@@ -575,7 +575,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course with an end date in the distant past is flagged.
      */
-    public function test_flag_course_past_enddate() {
+    public function test_flag_course_past_enddate(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -597,7 +597,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course with an end date in the distant past is flagged.
      */
-    public function test_flag_course_past_enddate_multiple() {
+    public function test_flag_course_past_enddate_multiple(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -625,7 +625,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course with an end date in the future is not flagged.
      */
-    public function test_flag_course_future_enddate() {
+    public function test_flag_course_future_enddate(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -644,7 +644,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course with an end date in the future is not flagged.
      */
-    public function test_flag_course_recent_unexpired_enddate() {
+    public function test_flag_course_recent_unexpired_enddate(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -662,7 +662,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course with an end date in the distant past is flagged, taking into account any purpose override
      */
-    public function test_flag_course_past_enddate_with_override_unexpired_role() {
+    public function test_flag_course_past_enddate_with_override_unexpired_role(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -701,7 +701,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course with an end date in the distant past is flagged, and any expired role is ignored.
      */
-    public function test_flag_course_past_enddate_with_override_expired_role() {
+    public function test_flag_course_past_enddate_with_override_expired_role(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -740,7 +740,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Ensure that where a course has explicitly expired one role, but that role is explicitly not expired in a child
      * context, does not have the parent context role expired.
      */
-    public function test_flag_course_override_expiredwith_override_unexpired_on_child() {
+    public function test_flag_course_override_expiredwith_override_unexpired_on_child(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -809,7 +809,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user context previously flagged as approved is not removed if the user has any unexpired roles.
      */
-    public function test_process_user_context_with_override_unexpired_role() {
+    public function test_process_user_context_with_override_unexpired_role(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -867,7 +867,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a module context previously flagged as approved is removed with appropriate unexpiredroles kept.
      */
-    public function test_process_course_context_with_override_unexpired_role() {
+    public function test_process_course_context_with_override_unexpired_role(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -955,7 +955,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a module context previously flagged as approved is removed with appropriate expiredroles kept.
      */
-    public function test_process_course_context_with_override_expired_role() {
+    public function test_process_course_context_with_override_expired_role(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -1043,7 +1043,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a module context previously flagged as approved is removed with appropriate expiredroles kept.
      */
-    public function test_process_course_context_with_user_in_both_lists() {
+    public function test_process_course_context_with_user_in_both_lists(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -1132,7 +1132,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a module context previously flagged as approved is removed with appropriate expiredroles kept.
      */
-    public function test_process_course_context_with_user_in_both_lists_expired() {
+    public function test_process_course_context_with_user_in_both_lists_expired(): void {
         global $DB;
         $this->resetAfterTest();
 
@@ -1228,7 +1228,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a site not setup will not process anything.
      */
-    public function test_process_not_setup() {
+    public function test_process_not_setup(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user(['lastaccess' => time() - YEARSECS]);
@@ -1265,7 +1265,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with no lastaccess is not flagged for deletion.
      */
-    public function test_process_none_approved() {
+    public function test_process_none_approved(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -1304,7 +1304,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with no lastaccess is not flagged for deletion.
      */
-    public function test_process_no_context() {
+    public function test_process_no_context(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -1343,7 +1343,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user context previously flagged as approved is removed.
      */
-    public function test_process_user_context() {
+    public function test_process_user_context(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -1405,7 +1405,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a course context previously flagged as approved is removed.
      */
-    public function test_process_course_context() {
+    public function test_process_course_context(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -1450,7 +1450,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user context previously flagged as approved is not removed if the user then logs in.
      */
-    public function test_process_user_context_logged_in_after_approval() {
+    public function test_process_user_context_logged_in_after_approval(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -1502,7 +1502,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user context previously flagged as approved is not removed if the purpose has changed.
      */
-    public function test_process_user_context_changed_after_approved() {
+    public function test_process_user_context_changed_after_approved(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -1554,7 +1554,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a historically expired expired block record child is cleaned up.
      */
-    public function test_process_user_historic_block_unapproved() {
+    public function test_process_user_historic_block_unapproved(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -1613,7 +1613,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that a user with a block which has a default retention period which has not expired, is still expired.
      */
-    public function test_process_user_historic_unexpired_child() {
+    public function test_process_user_historic_unexpired_child(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H');
@@ -1667,7 +1667,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Ensure that a course context previously flagged as approved for deletion which now has an unflagged child, is
      * updated.
      */
-    public function test_process_course_context_updated() {
+    public function test_process_course_context_updated(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -1718,7 +1718,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Ensure that a course context previously flagged as approved for deletion which now has an unflagged child, is
      * updated.
      */
-    public function test_process_course_context_outstanding_children() {
+    public function test_process_course_context_outstanding_children(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -1767,7 +1767,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Ensure that a course context previously flagged as approved for deletion which now has an unflagged child, is
      * updated.
      */
-    public function test_process_course_context_pending_children() {
+    public function test_process_course_context_pending_children(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -1825,7 +1825,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Ensure that a course context previously flagged as approved for deletion which now has an unflagged child, is
      * updated.
      */
-    public function test_process_course_context_approved_children() {
+    public function test_process_course_context_approved_children(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -1903,7 +1903,7 @@ class expired_contexts_test extends \advanced_testcase {
      * @param       int     $status
      * @param       bool    $expected
      */
-    public function test_can_process_deletion($status, $expected) {
+    public function test_can_process_deletion($status, $expected): void {
         $purpose = new expired_context(0, (object) [
             'status' => $status,
 
@@ -1942,7 +1942,7 @@ class expired_contexts_test extends \advanced_testcase {
      * @param       int     $status
      * @param       bool    $expected
      */
-    public function test_is_complete($status, $expected) {
+    public function test_is_complete($status, $expected): void {
         $purpose = new expired_context(0, (object) [
             'status' => $status,
             'contextid' => \context_system::instance()->id,
@@ -1980,7 +1980,7 @@ class expired_contexts_test extends \advanced_testcase {
      * @param       array   $record
      * @param       bool    $expected
      */
-    public function test_is_fully_expired($record, $expected) {
+    public function test_is_fully_expired($record, $expected): void {
         $purpose = new expired_context(0, (object) $record);
 
         $this->assertEquals($expected, $purpose->is_fully_expired());
@@ -2022,7 +2022,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that any orphaned records are removed once the context has been removed.
      */
-    public function test_orphaned_records_are_cleared() {
+    public function test_orphaned_records_are_cleared(): void {
         $this->resetAfterTest();
 
         $this->setup_basics('PT1H', 'PT1H', 'PT1H', 'PT1H');
@@ -2073,7 +2073,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that the progres tracer works as expected out of the box.
      */
-    public function test_progress_tracer_default() {
+    public function test_progress_tracer_default(): void {
         $manager = new \tool_dataprivacy\expired_contexts_manager();
 
         $rc = new \ReflectionClass(\tool_dataprivacy\expired_contexts_manager::class);
@@ -2085,7 +2085,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that the progres tracer works as expected when given a specific traer.
      */
-    public function test_progress_tracer_set() {
+    public function test_progress_tracer_set(): void {
         $manager = new \tool_dataprivacy\expired_contexts_manager();
         $mytrace = new \null_progress_trace();
         $manager->set_progress($mytrace);
@@ -2164,7 +2164,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with the system context.
      */
-    public function test_is_context_expired_system() {
+    public function test_is_context_expired_system(): void {
         $this->resetAfterTest();
         $this->setup_basics('PT1H', 'PT1H', 'P1D');
         $user = $this->getDataGenerator()->create_user(['lastaccess' => time() - YEARSECS]);
@@ -2180,7 +2180,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Children of a user context always follow the user expiry rather than any context level defaults (e.g. at the
      * block level.
      */
-    public function test_is_context_expired_user_block() {
+    public function test_is_context_expired_user_block(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1D');
@@ -2205,7 +2205,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with the front page course.
      */
-    public function test_is_context_expired_frontpage() {
+    public function test_is_context_expired_frontpage(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1D');
@@ -2239,7 +2239,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with an expired course.
      */
-    public function test_is_context_expired_course_expired() {
+    public function test_is_context_expired_course_expired(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1D');
@@ -2262,7 +2262,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with an unexpired course.
      */
-    public function test_is_context_expired_course_unexpired() {
+    public function test_is_context_expired_course_unexpired(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1D');
@@ -2291,7 +2291,7 @@ class expired_contexts_test extends \advanced_testcase {
      * If the course is still within the expiry period for the child context, then that child's protected flag should be
      * respected, even when the course may have expired.
      */
-    public function test_is_child_context_expired_course_unexpired_with_child() {
+    public function test_is_child_context_expired_course_unexpired_with_child(): void {
         $this->resetAfterTest();
 
         $purposes = $this->setup_basics('PT1H', 'PT1H', 'P1D', 'P1D');
@@ -2321,7 +2321,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with an expired course which has role overrides.
      */
-    public function test_is_context_expired_course_expired_override() {
+    public function test_is_context_expired_course_expired_override(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -2356,7 +2356,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with an expired course which has role overrides.
      */
-    public function test_is_context_expired_course_expired_override_parent() {
+    public function test_is_context_expired_course_expired_override_parent(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -2405,7 +2405,7 @@ class expired_contexts_test extends \advanced_testcase {
      * Test the is_context_expired functions when supplied with an expired course which has role overrides but the user
      * does not hold the role.
      */
-    public function test_is_context_expired_course_expired_override_parent_no_role() {
+    public function test_is_context_expired_course_expired_override_parent_no_role(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -2434,7 +2434,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with an unexpired course which has role overrides.
      */
-    public function test_is_context_expired_course_expired_override_inverse() {
+    public function test_is_context_expired_course_expired_override_inverse(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -2463,7 +2463,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with an unexpired course which has role overrides.
      */
-    public function test_is_context_expired_course_expired_override_inverse_parent() {
+    public function test_is_context_expired_course_expired_override_inverse_parent(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -2496,7 +2496,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Test the is_context_expired functions when supplied with an unexpired course which has role overrides.
      */
-    public function test_is_context_expired_course_expired_override_inverse_parent_not_assigned() {
+    public function test_is_context_expired_course_expired_override_inverse_parent_not_assigned(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -2528,7 +2528,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that context expired checks for a specific user taken into account roles.
      */
-    public function test_is_context_expired_or_unprotected_for_user_role_mixtures_protected() {
+    public function test_is_context_expired_or_unprotected_for_user_role_mixtures_protected(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -2583,7 +2583,7 @@ class expired_contexts_test extends \advanced_testcase {
     /**
      * Ensure that context expired checks for a specific user taken into account roles when retention is inversed.
      */
-    public function test_is_context_expired_or_unprotected_for_user_role_mixtures_protected_inverse() {
+    public function test_is_context_expired_or_unprotected_for_user_role_mixtures_protected_inverse(): void {
         global $DB;
 
         $this->resetAfterTest();

admin/tool/dataprivacy/tests/expired_data_requests_test.php

@@ -26,10 +26,11 @@ require_once('data_privacy_testcase.php');
  * Expired data requests tests.
  *
  * @package    tool_dataprivacy
+ * @covers     \tool_dataprivacy\data_request
  * @copyright  2018 Michael Hawkins
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class expired_data_requests_test extends data_privacy_testcase {
+final class expired_data_requests_test extends data_privacy_testcase {
 
     /**
      * Test tearDown.
@@ -41,7 +42,7 @@ class expired_data_requests_test extends data_privacy_testcase {
     /**
      * Test finding and deleting expired data requests
      */
-    public function test_data_request_expiry() {
+    public function test_data_request_expiry(): void {
         global $DB;
         $this->resetAfterTest();
         \core_privacy\local\request\writer::setup_real_writer_instance();
@@ -107,12 +108,58 @@ class expired_data_requests_test extends data_privacy_testcase {
         $this->assertEquals(0, $DB->count_records('files', $fileconditions));
     }
 
+    /**
+     * Test that data requests are not expired when expiration is disabled (set to zero)
+     */
+    public function test_data_request_expiry_never(): void {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        \core_privacy\local\request\writer::setup_real_writer_instance();
+
+        // Disable request expiry.
+        set_config('privacyrequestexpiry', 0, 'tool_dataprivacy');
+
+        // Create and approve data request.
+        $user = $this->getDataGenerator()->create_user();
+        $usercontext = \context_user::instance($user->id);
+
+        $this->setUser($user->id);
+        $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT);
+        $requestid = $datarequest->get('id');
+
+        $this->setAdminUser();
+        api::approve_data_request($requestid);
+
+        ob_start();
+        $this->runAdhocTasks('\tool_dataprivacy\task\process_data_request_task');
+        ob_end_clean();
+
+        // Run expiry deletion - should not affect test export.
+        $expiredrequests = data_request::get_expired_requests();
+        $this->assertEmpty($expiredrequests);
+        data_request::expire($expiredrequests);
+
+        // Confirm approved and exported.
+        $request = new data_request($requestid);
+        $this->assertEquals(api::DATAREQUEST_STATUS_DOWNLOAD_READY, $request->get('status'));
+        $fileconditions = [
+            'userid' => $user->id,
+            'component' => 'tool_dataprivacy',
+            'filearea' => 'export',
+            'itemid' => $requestid,
+            'contextid' => $usercontext->id,
+        ];
+        $this->assertEquals(2, $DB->count_records('files', $fileconditions));
+    }
+
     /**
      * Test for \tool_dataprivacy\data_request::is_expired()
      * Tests for the expected request status to protect from false positive/negative,
      * then tests is_expired() is returning the expected response.
      */
-    public function test_is_expired() {
+    public function test_is_expired(): void {
         $this->resetAfterTest();
         \core_privacy\local\request\writer::setup_real_writer_instance();
 

admin/tool/dataprivacy/tests/external/external_test.php

@@ -48,7 +48,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::approve_data_request() with the user not logged in.
      */
-    public function test_approve_data_request_not_logged_in() {
+    public function test_approve_data_request_not_logged_in(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -69,7 +69,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::approve_data_request() with the user not having a DPO role.
      */
-    public function test_approve_data_request_not_dpo() {
+    public function test_approve_data_request_not_dpo(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -89,7 +89,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::approve_data_request() for request that's not ready for approval
      */
-    public function test_approve_data_request_not_waiting_for_approval() {
+    public function test_approve_data_request_not_waiting_for_approval(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -110,7 +110,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::approve_data_request()
      */
-    public function test_approve_data_request() {
+    public function test_approve_data_request(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -133,7 +133,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::approve_data_request() for a non-existent request ID.
      */
-    public function test_approve_data_request_non_existent() {
+    public function test_approve_data_request_non_existent(): void {
         $this->resetAfterTest();
 
         // Admin as DPO. (The default when no one's assigned as a DPO in the site).
@@ -151,7 +151,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::cancel_data_request() of another user.
      */
-    public function test_cancel_data_request_other_user() {
+    public function test_cancel_data_request_other_user(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -177,7 +177,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test cancellation of a request where you are the requester of another user's data.
      */
-    public function test_cancel_data_request_other_user_as_requester() {
+    public function test_cancel_data_request_other_user_as_requester(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -204,7 +204,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test cancellation of a request where you are the requester of another user's data.
      */
-    public function test_cancel_data_request_requester_lost_permissions() {
+    public function test_cancel_data_request_requester_lost_permissions(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -233,7 +233,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test cancellation of a request where you are the requester of another user's data.
      */
-    public function test_cancel_data_request_other_user_as_child() {
+    public function test_cancel_data_request_other_user_as_child(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -260,7 +260,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::cancel_data_request()
      */
-    public function test_cancel_data_request() {
+    public function test_cancel_data_request(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -283,7 +283,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test contact DPO.
      */
-    public function test_contact_dpo() {
+    public function test_contact_dpo(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -300,7 +300,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test contact DPO with message containing invalid input.
      */
-    public function test_contact_dpo_with_nasty_input() {
+    public function test_contact_dpo_with_nasty_input(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -314,7 +314,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::deny_data_request() with the user not logged in.
      */
-    public function test_deny_data_request_not_logged_in() {
+    public function test_deny_data_request_not_logged_in(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -334,7 +334,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::deny_data_request() with the user not having a DPO role.
      */
-    public function test_deny_data_request_not_dpo() {
+    public function test_deny_data_request_not_dpo(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -353,7 +353,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::deny_data_request() for request that's not ready for approval
      */
-    public function test_deny_data_request_not_waiting_for_approval() {
+    public function test_deny_data_request_not_waiting_for_approval(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -373,7 +373,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::deny_data_request()
      */
-    public function test_deny_data_request() {
+    public function test_deny_data_request(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -395,7 +395,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::deny_data_request() for a non-existent request ID.
      */
-    public function test_deny_data_request_non_existent() {
+    public function test_deny_data_request_non_existent(): void {
         $this->resetAfterTest();
 
         // Admin as DPO. (The default when no one's assigned as a DPO in the site).
@@ -412,7 +412,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::get_data_request() with the user not logged in.
      */
-    public function test_get_data_request_not_logged_in() {
+    public function test_get_data_request_not_logged_in(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -430,7 +430,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::get_data_request() with the user not having a DPO role.
      */
-    public function test_get_data_request_not_dpo() {
+    public function test_get_data_request_not_dpo(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -450,7 +450,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::get_data_request()
      */
-    public function test_get_data_request() {
+    public function test_get_data_request(): void {
         $this->resetAfterTest();
 
         $generator = new \testing_data_generator();
@@ -475,7 +475,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::get_data_request() for a non-existent request ID.
      */
-    public function test_get_data_request_non_existent() {
+    public function test_get_data_request_non_existent(): void {
         $this->resetAfterTest();
 
         // Admin as DPO. (The default when no one's assigned as a DPO in the site).
@@ -488,7 +488,7 @@ class external_test extends externallib_advanced_testcase {
      * Test for \tool_dataprivacy\external::set_context_defaults()
      * when called by a user that doesn't have the manage registry capability.
      */
-    public function test_set_context_defaults_no_capability() {
+    public function test_set_context_defaults_no_capability(): void {
         $this->resetAfterTest();
 
         $generator = $this->getDataGenerator();
@@ -508,7 +508,7 @@ class external_test extends externallib_advanced_testcase {
      * @param bool $modulelevel Whether defaults are to be applied on the module context level or for an activity only.
      * @param bool $override Whether to override instances.
      */
-    public function test_set_context_defaults($modulelevel, $override) {
+    public function test_set_context_defaults($modulelevel, $override): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -567,7 +567,7 @@ class external_test extends externallib_advanced_testcase {
      * Test for \tool_dataprivacy\external::get_category_options()
      * when called by a user that doesn't have the manage registry capability.
      */
-    public function test_get_category_options_no_capability() {
+    public function test_get_category_options_no_capability(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user();
@@ -596,7 +596,7 @@ class external_test extends externallib_advanced_testcase {
      * @param bool $includeinherit Whether "Inherit" would be included to the options.
      * @param bool $includenotset Whether "Not set" would be included to the options.
      */
-    public function test_get_category_options($includeinherit, $includenotset) {
+    public function test_get_category_options($includeinherit, $includenotset): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -642,7 +642,7 @@ class external_test extends externallib_advanced_testcase {
      * Test for \tool_dataprivacy\external::get_purpose_options()
      * when called by a user that doesn't have the manage registry capability.
      */
-    public function test_get_purpose_options_no_capability() {
+    public function test_get_purpose_options_no_capability(): void {
         $this->resetAfterTest();
         $generator = $this->getDataGenerator();
         $user = $generator->create_user();
@@ -658,7 +658,7 @@ class external_test extends externallib_advanced_testcase {
      * @param bool $includeinherit Whether "Inherit" would be included to the options.
      * @param bool $includenotset Whether "Not set" would be included to the options.
      */
-    public function test_get_purpose_options($includeinherit, $includenotset) {
+    public function test_get_purpose_options($includeinherit, $includenotset): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -726,7 +726,7 @@ class external_test extends externallib_advanced_testcase {
      * @param bool $inheritpurpose Whether the purpose would be set to "Inherit".
      * @param bool $nodefaults Whether to fetch only activities that don't have defaults.
      */
-    public function test_get_activity_options($inheritcategory, $inheritpurpose, $nodefaults) {
+    public function test_get_activity_options($inheritcategory, $inheritpurpose, $nodefaults): void {
         $this->resetAfterTest();
         $this->setAdminUser();
 
@@ -774,7 +774,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::bulk_approve_data_requests().
      */
-    public function test_bulk_approve_data_requests() {
+    public function test_bulk_approve_data_requests(): void {
         $this->resetAfterTest();
 
         // Create delete data requests.
@@ -802,7 +802,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::bulk_approve_data_requests() for a non-existent request ID.
      */
-    public function test_bulk_approve_data_requests_non_existent() {
+    public function test_bulk_approve_data_requests_non_existent(): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -820,7 +820,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::bulk_deny_data_requests() for a user without permission to deny requests.
      */
-    public function test_bulk_approve_data_requests_no_permission() {
+    public function test_bulk_approve_data_requests_no_permission(): void {
         $this->resetAfterTest();
 
         // Create delete data requests.
@@ -849,7 +849,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::bulk_deny_data_requests() for a user without permission to deny requests.
      */
-    public function test_bulk_approve_data_requests_own_request() {
+    public function test_bulk_approve_data_requests_own_request(): void {
         $this->resetAfterTest();
 
         // Create delete data requests.
@@ -877,7 +877,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::bulk_deny_data_requests().
      */
-    public function test_bulk_deny_data_requests() {
+    public function test_bulk_deny_data_requests(): void {
         $this->resetAfterTest();
 
         // Create delete data requests.
@@ -905,7 +905,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::bulk_deny_data_requests() for a non-existent request ID.
      */
-    public function test_bulk_deny_data_requests_non_existent() {
+    public function test_bulk_deny_data_requests_non_existent(): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -922,7 +922,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::bulk_deny_data_requests() for a user without permission to deny requests.
      */
-    public function test_bulk_deny_data_requests_no_permission() {
+    public function test_bulk_deny_data_requests_no_permission(): void {
         $this->resetAfterTest();
 
         // Create delete data requests.
@@ -951,7 +951,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::bulk_deny_data_requests() for a user cannot approve their own request.
      */
-    public function test_bulk_deny_data_requests_own_request() {
+    public function test_bulk_deny_data_requests_own_request(): void {
         $this->resetAfterTest();
 
         // Create delete data requests.
@@ -986,7 +986,7 @@ class external_test extends externallib_advanced_testcase {
      * @throws required_capability_exception
      * @throws restricted_context_exception
      */
-    public function test_get_users_using_using_non_identity() {
+    public function test_get_users_using_using_non_identity(): void {
         $this->resetAfterTest();
         $context = \context_system::instance();
         $requester = $this->getDataGenerator()->create_user();
@@ -1021,7 +1021,7 @@ class external_test extends externallib_advanced_testcase {
      * @throws required_capability_exception
      * @throws restricted_context_exception
      */
-    public function test_get_users_using_identity_without_permission() {
+    public function test_get_users_using_identity_without_permission(): void {
         global $CFG;
 
         $this->resetAfterTest();
@@ -1053,7 +1053,7 @@ class external_test extends externallib_advanced_testcase {
      * @throws required_capability_exception
      * @throws restricted_context_exception
      */
-    public function test_get_users_using_field_not_in_identity() {
+    public function test_get_users_using_field_not_in_identity(): void {
         $this->resetAfterTest();
 
         $context = \context_system::instance();
@@ -1082,7 +1082,7 @@ class external_test extends externallib_advanced_testcase {
      * @throws required_capability_exception
      * @throws restricted_context_exception
      */
-    public function test_get_users() {
+    public function test_get_users(): void {
         global $CFG;
         $this->resetAfterTest();
         $CFG->showuseridentity = 'institution';
@@ -1118,7 +1118,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::get_access_information().
      */
-    public function test_get_access_information() {
+    public function test_get_access_information(): void {
         $this->resetAfterTest();
 
         $this->setAdminUser();
@@ -1145,7 +1145,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::create_data_request()
      */
-    public function test_create_data_request() {
+    public function test_create_data_request(): void {
         $this->resetAfterTest();
 
         $systemcontext = \context_system::instance();
@@ -1200,7 +1200,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::create_data_request() when no dpo available.
      */
-    public function test_create_data_request_no_dpo() {
+    public function test_create_data_request_no_dpo(): void {
         $this->resetAfterTest();
 
         $user = $this->getDataGenerator()->create_user();
@@ -1214,7 +1214,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::create_data_request() with missing permission.
      */
-    public function test_create_data_request_no_permission() {
+    public function test_create_data_request_no_permission(): void {
         $this->resetAfterTest();
 
         // Enable contact DPO.
@@ -1231,7 +1231,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::create_data_request() with invalid request type.
      */
-    public function test_create_data_request_invalid_type() {
+    public function test_create_data_request_invalid_type(): void {
         $this->resetAfterTest();
 
         // Enable contact DPO.
@@ -1248,7 +1248,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::get_data_requests().
      */
-    public function test_get_data_requests() {
+    public function test_get_data_requests(): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -1355,7 +1355,7 @@ class external_test extends externallib_advanced_testcase {
     /**
      * Test for external::get_data_requests() invalid user id.
      */
-    public function test_get_data_requests_invalid_userid() {
+    public function test_get_data_requests_invalid_userid(): void {
         $this->resetAfterTest();
         $this->setAdminUser();