Merge branch 'MDL-66118-master-byebyemoodlenet' of git://github.com/mudrd8mz/moodle
authorAndrew Nicols <andrew@nicols.co.uk>
Tue, 8 Oct 2019 23:43:33 +0000 (07:43 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Tue, 8 Oct 2019 23:43:33 +0000 (07:43 +0800)
61 files changed:
admin/settings/appearance.php
admin/tool/analytics/classes/output/insights_report.php [moved from admin/tool/analytics/classes/output/effectiveness_report.php with 96% similarity]
admin/tool/analytics/classes/output/models_list.php
admin/tool/analytics/classes/output/renderer.php
admin/tool/analytics/lang/en/tool_analytics.php
admin/tool/analytics/model.php
admin/tool/analytics/templates/insights_report.mustache [moved from admin/tool/analytics/templates/effectiveness_report.mustache with 90% similarity]
admin/tool/mobile/classes/api.php
admin/tool/mobile/classes/external.php
admin/tool/mobile/lang/en/tool_mobile.php
admin/tool/mobile/settings.php
admin/tool/mobile/tests/externallib_test.php
admin/tool/monitor/lib.php
backup/controller/backup_controller.class.php
backup/moodle2/backup_plugin.class.php
backup/moodle2/backup_stepslib.php
backup/moodle2/restore_plugin.class.php
backup/moodle2/restore_stepslib.php
backup/util/plan/restore_structure_step.class.php
cache/stores/redis/addinstanceform.php
cache/stores/redis/lang/en/cachestore_redis.php
cache/stores/redis/lib.php
cache/stores/redis/tests/compressor_test.php [new file with mode: 0644]
cache/upgrade.txt
customfield/tests/api_test.php
customfield/tests/category_controller_test.php
customfield/tests/data_controller_test.php
customfield/tests/field_controller_test.php
customfield/tests/generator_test.php
customfield/tests/privacy_test.php
enrol/tests/enrollib_test.php
install/lang/nl/install.php
lang/en/admin.php
lib/classes/hub/api.php
lib/classes/hub/registration.php
lib/classes/hub/site_registration_form.php
lib/db/upgrade.php
lib/enrollib.php
lib/form/templates/element-select-inline.mustache
lib/form/templates/element-select.mustache
lib/form/templates/element-selectgroups-inline.mustache
lib/form/templates/element-selectgroups.mustache
lib/form/templates/element-selectwithlink.mustache
message/templates/message_drawer_view_conversation_body.mustache
mod/quiz/attemptlib.php
mod/quiz/tests/behat/attempt_redo_questions.feature
mod/quiz/tests/behat/backup.feature
mod/quiz/tests/behat/behat_mod_quiz.php
question/engine/datalib.php
question/engine/questionusage.php
question/engine/states.php
question/engine/tests/datalib_reporting_queries_test.php
question/format.php
question/format/xml/format.php
question/format/xml/tests/fixtures/category_with_description.xml
question/format/xml/tests/fixtures/export_category.xml
question/format/xml/tests/fixtures/nested_categories.xml
question/format/xml/tests/fixtures/nested_categories_with_questions.xml
question/format/xml/tests/qformat_xml_import_export_test.php
question/format/xml/tests/xmlformat_test.php
version.php

index dc924f7..c9d406a 100644 (file)
@@ -197,6 +197,10 @@ preferences,moodle|/user/preferences.php|t/preferences',
         'idnumber' => new lang_string('sort_idnumber', 'admin'),
     );
     $temp->add(new admin_setting_configselect('navsortmycoursessort', new lang_string('navsortmycoursessort', 'admin'), new lang_string('navsortmycoursessort_help', 'admin'), 'sortorder', $sortoptions));
+    $temp->add(new admin_setting_configcheckbox('navsortmycourseshiddenlast',
+            new lang_string('navsortmycourseshiddenlast', 'admin'),
+            new lang_string('navsortmycourseshiddenlast_help', 'admin'),
+            1));
     $temp->add(new admin_setting_configtext('navcourselimit', new lang_string('navcourselimit', 'admin'),
         new lang_string('confignavcourselimit', 'admin'), 10, PARAM_INT));
     $temp->add(new admin_setting_configcheckbox('usesitenameforsitepages', new lang_string('usesitenameforsitepages', 'admin'), new lang_string('configusesitenameforsitepages', 'admin'), 0));
@@ -15,7 +15,7 @@
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
 /**
- * Effectiveness report renderable.
+ * Insights report renderable.
  *
  * @package    tool_analytics
  * @copyright  2019 David Monllao {@link http://www.davidmonllao.com}
@@ -27,13 +27,13 @@ namespace tool_analytics\output;
 defined('MOODLE_INTERNAL') || die;
 
 /**
- * Effectiveness report renderable.
+ * Insights report renderable.
  *
  * @package    tool_analytics
  * @copyright  2019 David Monllao {@link http://www.davidmonllao.com}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class effectiveness_report implements \renderable, \templatable {
+class insights_report implements \renderable, \templatable {
 
     /**
      * @var \core_analytics\model
@@ -46,7 +46,7 @@ class effectiveness_report implements \renderable, \templatable {
     private $context = null;
 
     /**
-     * Inits the effectiveness report renderable.
+     * Inits the insights report renderable.
      *
      * @param \core_analytics\model $model
      * @param int|null $contextid
@@ -79,7 +79,7 @@ class effectiveness_report implements \renderable, \templatable {
         $predictioncontexts = $this->model->get_predictions_contexts(false);
         if ($predictioncontexts && count($predictioncontexts) > 1) {
             $url = new \moodle_url('/admin/tool/analytics/model.php', ['id' => $this->model->get_id(),
-                'action' => 'effectivenessreport']);
+                'action' => 'insightsreport']);
 
             if ($this->context) {
                 $selected = $this->context->id;
index be1c7ba..ade6739 100644 (file)
@@ -288,12 +288,12 @@ class models_list implements \renderable, \templatable {
                 }
             }
 
-            // Effectivity report.
+            // Insights report.
             if (!empty($anypredictionobtained) && $model->uses_insights()) {
-                $urlparams['action'] = 'effectivenessreport';
+                $urlparams['action'] = 'insightsreport';
                 $url = new \moodle_url('/admin/tool/analytics/model.php', $urlparams);
-                $pix = new \pix_icon('i/report', get_string('effectivenessreport', 'tool_analytics'));
-                $icon = new \action_menu_link_secondary($url, $pix, get_string('effectivenessreport', 'tool_analytics'));
+                $pix = new \pix_icon('i/report', get_string('insightsreport', 'tool_analytics'));
+                $icon = new \action_menu_link_secondary($url, $pix, get_string('insightsreport', 'tool_analytics'));
                 $actionsmenu->add($icon);
             }
 
index 60bc2a2..b4f7a51 100644 (file)
@@ -211,12 +211,12 @@ class renderer extends plugin_renderer_base {
     /**
      * Defer to template.
      *
-     * @param \tool_analytics\output\effectiveness_report $effectivenessreport
+     * @param \tool_analytics\output\insights_report $insightsreport
      * @return string HTML
      */
-    protected function render_effectiveness_report(\tool_analytics\output\effectiveness_report $effectivenessreport): string {
-        $data = $effectivenessreport->export_for_template($this);
-        return parent::render_from_template('tool_analytics/effectiveness_report', $data);
+    protected function render_insights_report(\tool_analytics\output\insights_report $insightsreport): string {
+        $data = $insightsreport->export_for_template($this);
+        return parent::render_from_template('tool_analytics/insights_report', $data);
     }
 
     /**
index 7041b83..ca3d1a2 100644 (file)
@@ -25,6 +25,7 @@
 $string['accuracy'] = 'Accuracy';
 $string['actions'] = 'Actions';
 $string['actionsexecutedbyusers'] = 'Actions executed by users';
+$string['actionsexecutedbyusersfor'] = 'Actions executed by users for "{$a}" model';
 $string['actionexecutedgroupedusefulness'] = 'Grouped actions';
 $string['allpredictions'] = 'All predictions';
 $string['alltimesplittingmethods'] = 'All analysis intervals';
@@ -51,8 +52,6 @@ $string['deletemodelconfirmation'] = 'Are you sure you want to delete "{$a}"? Th
 $string['disabled'] = 'Disabled';
 $string['editmodel'] = 'Edit "{$a}" model';
 $string['edittrainedwarning'] = 'This model has already been trained. Note that changing its indicators or its analysis interval will delete its previous predictions and start generating new predictions.';
-$string['effectivenessreport'] = 'Effectiveness report';
-$string['effectivenessreportfor'] = 'Model "{$a}" effectiveness';
 $string['enabled'] = 'Enabled';
 $string['errorcantenablenotimesplitting'] = 'You need to select an analysis interval before enabling the model';
 $string['errornoenabledandtrainedmodels'] = 'There are no enabled and trained models to predict.';
@@ -91,6 +90,7 @@ $string['indicators_help'] = 'The indicators are what you think will lead to an
 $string['indicators_link'] = 'Indicators';
 $string['indicatorsnum'] = 'Number of indicators: {$a}';
 $string['info'] = 'Info';
+$string['insightsreport'] = 'Insights report';
 $string['ignoreversionmismatches'] = 'Ignore version mismatches';
 $string['ignoreversionmismatchescheckbox'] = 'Ignore the differences between this site version and the original site version.';
 $string['importedsuccessfully'] = 'The model has been successfully imported.';
index 653ab82..7397de1 100644 (file)
@@ -69,8 +69,8 @@ switch ($action) {
     case 'clear':
         $title = get_string('clearpredictions', 'tool_analytics');
         break;
-    case 'effectivenessreport':
-        $title = get_string('effectivenessreport', 'tool_analytics');
+    case 'insightsreport':
+        $title = get_string('insightsreport', 'tool_analytics');
         break;
     case 'invalidanalysables':
         $title = get_string('invalidanalysables', 'tool_analytics');
@@ -282,13 +282,13 @@ switch ($action) {
         redirect($returnurl);
         break;
 
-    case 'effectivenessreport':
+    case 'insightsreport':
 
         $contextid = optional_param('contextid', null, PARAM_INT);
 
         echo $OUTPUT->header();
 
-        $renderable = new \tool_analytics\output\effectiveness_report($model, $contextid);
+        $renderable = new \tool_analytics\output\insights_report($model, $contextid);
         $renderer = $PAGE->get_renderer('tool_analytics');
         echo $renderer->render($renderable);
 
@@ -15,9 +15,9 @@
     along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 }}
 {{!
-    @template tool_analytics/effectiveness_report
+    @template tool_analytics/insights_report
 
-    Template for the effectiveness report.
+    Template for the insights report.
 
     Classes required for JS:
     * none
@@ -39,7 +39,7 @@
 }}
 
 <div class="box">
-    <h3>{{#str}}effectivenessreportfor, tool_analytics, {{modelname}}{{/str}}</h3>
+    <h3>{{#str}}actionsexecutedbyusersfor, tool_analytics, {{modelname}}{{/str}}</h3>
 
     {{#contextselect}}
         <div class="mt-3">
index 978f950..1b34498 100644 (file)
@@ -177,6 +177,10 @@ class api {
             'langmenu' => $CFG->langmenu,
             'langlist' => $CFG->langlist,
             'locale' => $CFG->locale,
+            'tool_mobile_minimumversion' => get_config('tool_mobile', 'minimumversion'),
+            'tool_mobile_iosappid' => get_config('tool_mobile', 'iosappid'),
+            'tool_mobile_androidappid' => get_config('tool_mobile', 'androidappid'),
+            'tool_mobile_setuplink' => clean_param(get_config('tool_mobile', 'setuplink'), PARAM_URL),
         );
 
         $typeoflogin = get_config('tool_mobile', 'typeoflogin');
index 73c81ff..fe1dad0 100644 (file)
@@ -179,6 +179,13 @@ class external extends external_api {
                 'langmenu' => new external_value(PARAM_INT, 'Whether the language menu should be displayed.', VALUE_OPTIONAL),
                 'langlist' => new external_value(PARAM_RAW, 'Languages on language menu.', VALUE_OPTIONAL),
                 'locale' => new external_value(PARAM_RAW, 'Sitewide locale.', VALUE_OPTIONAL),
+                'tool_mobile_minimumversion' => new external_value(PARAM_NOTAGS, 'Minimum required version to access.',
+                    VALUE_OPTIONAL),
+                'tool_mobile_iosappid' => new external_value(PARAM_ALPHANUM, 'iOS app\'s unique identifier.',
+                    VALUE_OPTIONAL),
+                'tool_mobile_androidappid' => new external_value(PARAM_NOTAGS, 'Android app\'s unique identifier.',
+                    VALUE_OPTIONAL),
+                'tool_mobile_setuplink' => new external_value(PARAM_URL, 'App download page.', VALUE_OPTIONAL),
                 'warnings' => new external_warnings(),
             )
         );
index b02ff9e..5c7686a 100644 (file)
@@ -75,6 +75,8 @@ $string['loginintheapp'] = 'Via the app';
 $string['logininthebrowser'] = 'Via a browser window (for SSO plugins)';
 $string['loginintheembeddedbrowser'] = 'Via an embedded browser (for SSO plugins)';
 $string['mainmenu'] = 'Main menu';
+$string['minimumversion'] = 'Require users to upgrade their apps to the minimum version indicated. Those using previous versions of the app will not be able to access to the site. This works since app version 3.8.0 onward.';
+$string['minimumversion_key'] = 'Minimum app version required';
 $string['mobileapp'] = 'Mobile app';
 $string['mobileappconnected'] = 'Mobile app connected';
 $string['mobileappenabled'] = 'This site has mobile app access enabled.<br /><a href="{$a}">Download the mobile app</a>.';
index b5033d8..2121e84 100644 (file)
@@ -65,6 +65,10 @@ if ($hassiteconfig) {
                     new lang_string('forcedurlscheme_key', 'tool_mobile'),
                     new lang_string('forcedurlscheme', 'tool_mobile'), 'moodlemobile', PARAM_ALPHANUM));
 
+        $temp->add(new admin_setting_configtext('tool_mobile/minimumversion',
+                    new lang_string('minimumversion_key', 'tool_mobile'),
+                    new lang_string('minimumversion', 'tool_mobile'), '', PARAM_NOTAGS));
+
         $ADMIN->add('mobileapp', $temp);
 
         // Appearance related settings.
index 955fbb9..c2118dc 100644 (file)
@@ -95,6 +95,10 @@ class tool_mobile_external_testcase extends externallib_advanced_testcase {
             'langmenu' => $CFG->langmenu,
             'langlist' => $CFG->langlist,
             'locale' => $CFG->locale,
+            'tool_mobile_minimumversion' => '',
+            'tool_mobile_iosappid' => get_config('tool_mobile', 'iosappid'),
+            'tool_mobile_androidappid' => get_config('tool_mobile', 'androidappid'),
+            'tool_mobile_setuplink' => get_config('tool_mobile', 'setuplink'),
             'warnings' => array()
         );
         $this->assertEquals($expected, $result);
@@ -111,6 +115,7 @@ class tool_mobile_external_testcase extends externallib_advanced_testcase {
         set_config('autolang', 1);
         set_config('lang', 'a_b');  // Set invalid lang.
         set_config('disabledfeatures', 'myoverview', 'tool_mobile');
+        set_config('minimumversion', '3.8.0', 'tool_mobile');
 
         list($authinstructions, $notusedformat) = external_format_text($authinstructions, FORMAT_MOODLE, $context->id);
         $expected['registerauth'] = 'email';
@@ -123,6 +128,7 @@ class tool_mobile_external_testcase extends externallib_advanced_testcase {
         $expected['autolang'] = '1';
         $expected['lang'] = ''; // Expect empty because it was set to an invalid lang.
         $expected['tool_mobile_disabledfeatures'] = 'myoverview';
+        $expected['tool_mobile_minimumversion'] = '3.8.0';
 
         if ($logourl = $OUTPUT->get_logo_url()) {
             $expected['logourl'] = $logourl->out(false);
index 58078ec..df11ac9 100644 (file)
@@ -119,7 +119,9 @@ function tool_monitor_can_subscribe() {
  * @return array|bool Returns an array of courses or false if the user has no permission to subscribe to rules.
  */
 function tool_monitor_get_user_courses() {
-    $orderby = 'visible DESC, sortorder ASC';
+    // Get the course sorting according to the admin settings.
+    $sort = enrol_get_courses_sortingsql();
+
     $options = array();
     if (has_capability('tool/monitor:subscribe', context_system::instance())) {
         $options[0] = get_string('site');
@@ -134,7 +136,7 @@ function tool_monitor_get_user_courses() {
         );
 
     $fields = implode(', ', $fieldlist);
-    if ($courses = get_user_capability_course('tool/monitor:subscribe', null, true, $fields, $orderby)) {
+    if ($courses = get_user_capability_course('tool/monitor:subscribe', null, true, $fields, $sort)) {
         foreach ($courses as $course) {
             context_helper::preload_from_record($course);
             $coursectx = context_course::instance($course->id);
index cf5fcf9..52793b2 100644 (file)
@@ -55,6 +55,7 @@ class backup_controller extends base_controller {
 
     protected $status; // Current status of the controller (created, planned, configured...)
 
+    /** @var backup_plan */
     protected $plan;   // Backup execution plan
     protected $includefiles; // Whether this backup includes files or not.
 
index 6a060ab..32f6c1d 100644 (file)
@@ -34,13 +34,27 @@ defined('MOODLE_INTERNAL') || die();
  */
 abstract class backup_plugin {
 
+    /** @var string */
     protected $plugintype;
+    /** @var string */
     protected $pluginname;
+    /** @var string */
     protected $connectionpoint;
+    /** @var backup_optigroup_element */
     protected $optigroup; // Optigroup, parent of all optigroup elements
+    /** @var backup_structure_step */
     protected $step;
+    /** @var backup_course_task|backup_activity_task */
     protected $task;
 
+    /**
+     * backup_plugin constructor.
+     *
+     * @param string $plugintype
+     * @param string $pluginname
+     * @param backup_optigroup_element $optigroup
+     * @param backup_structure_step $step
+     */
     public function __construct($plugintype, $pluginname, $optigroup, $step) {
         $this->plugintype = $plugintype;
         $this->pluginname = $pluginname;
index 11dd4ce..ac8c917 100644 (file)
@@ -125,10 +125,9 @@ abstract class backup_activity_structure_step extends backup_structure_step {
 }
 
 /**
- * Abstract structure step, to be used by all the activities using core questions stuff
- * (namely quiz module), supporting question plugins, states and sessions
+ * Helper code for use by any plugin that stores question attempt data that it needs to back up.
  */
-abstract class backup_questions_activity_structure_step extends backup_activity_structure_step {
+trait backup_questions_attempt_data_trait {
 
     /**
      * Attach to $element (usually attempts) the needed backup structures
@@ -200,6 +199,17 @@ abstract class backup_questions_activity_structure_step extends backup_activity_
 }
 
 
+/**
+ * Abstract structure step to help activities that store question attempt data.
+ *
+ * @copyright 2011 The Open University
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+abstract class backup_questions_activity_structure_step extends backup_activity_structure_step {
+    use backup_questions_attempt_data_trait;
+}
+
+
 /**
  * backup structure step in charge of calculating the categories to be
  * included in backup, based in the context being backuped (module/course)
@@ -275,6 +285,9 @@ class backup_module_structure_step extends backup_structure_step {
         // attach format plugin structure to $module element, only one allowed
         $this->add_plugin_structure('format', $module, false);
 
+        // Attach report plugin structure to $module element, multiple allowed.
+        $this->add_plugin_structure('report', $module, true);
+
         // attach plagiarism plugin structure to $module element, there can be potentially
         // many plagiarism plugins storing information about this course
         $this->add_plugin_structure('plagiarism', $module, true);
index b90bcdf..31a94f5 100644 (file)
@@ -34,12 +34,24 @@ defined('MOODLE_INTERNAL') || die();
  */
 abstract class restore_plugin {
 
+    /** @var string */
     protected $plugintype;
+    /** @var string */
     protected $pluginname;
+    /** @var string */
     protected $connectionpoint;
+    /** @var restore_structure_step */
     protected $step;
+    /** @var restore_course_task|restore_activity_task */
     protected $task;
 
+    /**
+     * restore_plugin constructor.
+     *
+     * @param string $plugintype
+     * @param string $pluginname
+     * @param restore_structure_step $step
+     */
     public function __construct($plugintype, $pluginname, $step) {
         $this->plugintype = $plugintype;
         $this->pluginname = $pluginname;
@@ -58,9 +70,11 @@ abstract class restore_plugin {
         $methodname = 'define_' . basename($this->connectionpoint->get_path()) . '_plugin_structure';
 
         if (method_exists($this, $methodname)) {
-            if ($bluginpaths = $this->$methodname()) {
-                foreach ($bluginpaths as $path) {
-                    $path->set_processing_object($this);
+            if ($pluginpaths = $this->$methodname()) {
+                foreach ($pluginpaths as $path) {
+                    if ($path->get_processing_object() === null && !$this->step->grouped_parent_exists($path, $paths)) {
+                        $path->set_processing_object($this);
+                    }
                     $paths[] = $path;
                 }
             }
@@ -258,4 +272,13 @@ abstract class restore_plugin {
                'plugin_' . $this->plugintype . '_' .
                $this->pluginname . '_' . basename($this->connectionpoint->get_path()) . $path;
     }
+
+    /**
+     * Get the task we are part of.
+     *
+     * @return restore_activity_task|restore_course_task the task.
+     */
+    protected function get_task() {
+        return $this->task;
+    }
 }
index 0d6518d..10199e0 100644 (file)
@@ -4180,6 +4180,9 @@ class restore_module_structure_step extends restore_structure_step {
         // Apply for 'format' plugins optional paths at module level
         $this->add_plugin_structure('format', $module);
 
+        // Apply for 'report' plugins optional paths at module level.
+        $this->add_plugin_structure('report', $module);
+
         // Apply for 'plagiarism' plugins optional paths at module level
         $this->add_plugin_structure('plagiarism', $module);
 
@@ -5323,10 +5326,9 @@ class restore_process_file_aliases_queue extends restore_execution_step {
 
 
 /**
- * Abstract structure step, to be used by all the activities using core questions stuff
- * (like the quiz module), to support qtype plugins, states and sessions
+ * Helper code for use by any plugin that stores question attempt data that it needs to back up.
  */
-abstract class restore_questions_activity_structure_step extends restore_activity_structure_step {
+trait restore_questions_attempt_data_trait {
     /** @var array question_attempt->id to qtype. */
     protected $qtypes = array();
     /** @var array question_attempt->id to questionid. */
@@ -5378,21 +5380,21 @@ abstract class restore_questions_activity_structure_step extends restore_activit
     /**
      * Process question_usages
      */
-    protected function process_question_usage($data) {
+    public function process_question_usage($data) {
         $this->restore_question_usage_worker($data, '');
     }
 
     /**
      * Process question_attempts
      */
-    protected function process_question_attempt($data) {
+    public function process_question_attempt($data) {
         $this->restore_question_attempt_worker($data, '');
     }
 
     /**
      * Process question_attempt_steps
      */
-    protected function process_question_attempt_step($data) {
+    public function process_question_attempt_step($data) {
         $this->restore_question_attempt_step_worker($data, '');
     }
 
@@ -5412,8 +5414,7 @@ abstract class restore_questions_activity_structure_step extends restore_activit
         $data = (object)$data;
         $oldid = $data->id;
 
-        $oldcontextid = $this->get_task()->get_old_contextid();
-        $data->contextid  = $this->get_mappingid('context', $this->task->get_old_contextid());
+        $data->contextid  = $this->task->get_contextid();
 
         // Everything ready, insert (no mapping needed)
         $newitemid = $DB->insert_record('question_usages', $data);
@@ -5569,6 +5570,17 @@ abstract class restore_questions_activity_structure_step extends restore_activit
             $this->add_related_files('question', $filearea, 'question_attempt_step');
         }
     }
+}
+
+
+/**
+ * Abstract structure step to help activities that store question attempt data.
+ *
+ * @copyright 2011 The Open University
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+abstract class restore_questions_activity_structure_step extends restore_activity_structure_step {
+    use restore_questions_attempt_data_trait;
 
     /**
      * Attach below $element (usually attempts) the needed restore_path_elements
@@ -5612,7 +5624,7 @@ abstract class restore_questions_activity_structure_step extends restore_activit
     /**
      * Process the attempt data defined by {@link add_legacy_question_attempt_data()}.
      * @param object $data contains all the grouped attempt data to process.
-     * @param pbject $quiz data about the activity the attempts belong to. Required
+     * @param object $quiz data about the activity the attempts belong to. Required
      * fields are (basically this only works for the quiz module):
      *      oldquestions => list of question ids in this activity - using old ids.
      *      preferredbehaviour => the behaviour to use for questionattempts.
@@ -5674,7 +5686,7 @@ abstract class restore_questions_activity_structure_step extends restore_activit
 
         $data->uniqueid = $usage->id;
         $upgrader->save_usage($quiz->preferredbehaviour, $data, $qas,
-                 $this->questions_recode_layout($quiz->oldquestions));
+                $this->questions_recode_layout($quiz->oldquestions));
     }
 
     protected function find_question_session_and_states($data, $questionid) {
@@ -5734,6 +5746,7 @@ abstract class restore_questions_activity_structure_step extends restore_activit
     }
 }
 
+
 /**
  * Restore completion defaults for each module type
  *
index bffc7b7..3fe1978 100644 (file)
@@ -496,9 +496,9 @@ abstract class restore_structure_step extends restore_step {
         // Now, for each element not having one processing object, if
         // not child of grouped element, assign $this (the step itself) as processing element
         // Note method must exist or we'll get one @restore_path_element_exception
-        foreach($paths as $key => $pelement) {
+        foreach ($paths as $pelement) {
             if ($pelement->get_processing_object() === null && !$this->grouped_parent_exists($pelement, $paths)) {
-                $paths[$key]->set_processing_object($this);
+                $pelement->set_processing_object($this);
             }
             // Populate $elementsoldid and $elementsoldid based on available pathelements
             $this->elementsoldid[$pelement->get_name()] = null;
@@ -510,18 +510,22 @@ abstract class restore_structure_step extends restore_step {
 
     /**
      * Given one pathelement, return true if grouped parent was found
+     *
+     * @param restore_path_element $pelement the element we are interested in.
+     * @param restore_path_element[] $elements the elements that exist.
+     * @return bool true if this element is inside a grouped parent.
      */
-    protected function grouped_parent_exists($pelement, $elements) {
+    public function grouped_parent_exists($pelement, $elements) {
         foreach ($elements as $element) {
             if ($pelement->get_path() == $element->get_path()) {
-                continue; // Don't compare against itself
+                continue; // Don't compare against itself.
             }
-            // If element is grouped and parent of pelement, return true
+            // If element is grouped and parent of pelement, return true.
             if ($element->is_grouped() and strpos($pelement->get_path() .  '/', $element->get_path()) === 0) {
                 return true;
             }
         }
-        return false; // no grouped parent found
+        return false; // No grouped parent found.
     }
 
     /**
index ad9bc00..1a3faa1 100644 (file)
@@ -58,5 +58,11 @@ class cachestore_redis_addinstance_form extends cachestore_addinstance_form {
         $form->addHelpButton('serializer', 'useserializer', 'cachestore_redis');
         $form->setDefault('serializer', Redis::SERIALIZER_PHP);
         $form->setType('serializer', PARAM_INT);
+
+        $compressoroptions = cachestore_redis::config_get_compressor_options();
+        $form->addElement('select', 'compressor', get_string('usecompressor', 'cachestore_redis'), $compressoroptions);
+        $form->addHelpButton('compressor', 'usecompressor', 'cachestore_redis');
+        $form->setDefault('compressor', cachestore_redis::COMPRESSOR_NONE);
+        $form->setType('compressor', PARAM_INT);
     }
 }
index 0d155be..a52b93f 100644 (file)
@@ -24,6 +24,9 @@
 
 defined('MOODLE_INTERNAL') || die();
 
+$string['compressor_none'] = 'No compression.';
+$string['compressor_php_gzip'] = 'Use gzip compression.';
+$string['compressor_php_zstd'] = 'Use Zstandard compression.';
 $string['pluginname'] = 'Redis';
 $string['prefix'] = 'Key prefix';
 $string['prefix_help'] = 'This prefix is used for all key names on the Redis server.
@@ -48,3 +51,5 @@ $string['useserializer'] = 'Use serializer';
 $string['useserializer_help'] = 'Specifies the serializer to use for serializing.
 The valid serializers are Redis::SERIALIZER_PHP or Redis::SERIALIZER_IGBINARY.
 The latter is supported only when phpredis is configured with --enable-redis-igbinary option and the igbinary extension is loaded.';
+$string['usecompressor'] = 'Use compressor';
+$string['usecompressor_help'] = 'Specifies the compressor to use after serializing. It is done at Moodle Cache API level, not at php-redis level.';
index 3596200..bde6b9e 100644 (file)
@@ -38,6 +38,21 @@ defined('MOODLE_INTERNAL') || die();
  */
 class cachestore_redis extends cache_store implements cache_is_key_aware, cache_is_lockable,
         cache_is_configurable, cache_is_searchable {
+    /**
+     * Compressor: none.
+     */
+    const COMPRESSOR_NONE = 0;
+
+    /**
+     * Compressor: PHP GZip.
+     */
+    const COMPRESSOR_PHP_GZIP = 1;
+
+    /**
+     * Compressor: PHP Zstandard.
+     */
+    const COMPRESSOR_PHP_ZSTD = 2;
+
     /**
      * Name of this store.
      *
@@ -80,6 +95,13 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
      */
     protected $serializer = Redis::SERIALIZER_PHP;
 
+    /**
+     * Compressor for this store.
+     *
+     * @var int
+     */
+    protected $compressor = self::COMPRESSOR_NONE;
+
     /**
      * Determines if the requirements for this type of store are met.
      *
@@ -134,6 +156,9 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
         if (array_key_exists('serializer', $configuration)) {
             $this->serializer = (int)$configuration['serializer'];
         }
+        if (array_key_exists('compressor', $configuration)) {
+            $this->compressor = (int)$configuration['compressor'];
+        }
         $password = !empty($configuration['password']) ? $configuration['password'] : '';
         $prefix = !empty($configuration['prefix']) ? $configuration['prefix'] : '';
         $this->redis = $this->new_redis($configuration['server'], $prefix, $password);
@@ -161,7 +186,10 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
             if (!empty($password)) {
                 $redis->auth($password);
             }
-            $redis->setOption(Redis::OPT_SERIALIZER, $this->serializer);
+            // If using compressor, serialisation will be done at cachestore level, not php-redis.
+            if ($this->compressor == self::COMPRESSOR_NONE) {
+                $redis->setOption(Redis::OPT_SERIALIZER, $this->serializer);
+            }
             if (!empty($prefix)) {
                 $redis->setOption(Redis::OPT_PREFIX, $prefix);
             }
@@ -236,7 +264,13 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
      * @return mixed The value of the key, or false if there is no value associated with the key.
      */
     public function get($key) {
-        return $this->redis->hGet($this->hash, $key);
+        $value = $this->redis->hGet($this->hash, $key);
+
+        if ($this->compressor == self::COMPRESSOR_NONE) {
+            return $value;
+        }
+
+        return $this->uncompress($value);
     }
 
     /**
@@ -246,7 +280,17 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
      * @return array An array of the values of the given keys.
      */
     public function get_many($keys) {
-        return $this->redis->hMGet($this->hash, $keys);
+        $values = $this->redis->hMGet($this->hash, $keys);
+
+        if ($this->compressor == self::COMPRESSOR_NONE) {
+            return $values;
+        }
+
+        foreach ($values as &$value) {
+            $value = $this->uncompress($value);
+        }
+
+        return $values;
     }
 
     /**
@@ -257,6 +301,10 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
      * @return bool True if the operation succeeded, false otherwise.
      */
     public function set($key, $value) {
+        if ($this->compressor != self::COMPRESSOR_NONE) {
+            $value = $this->compress($value);
+        }
+
         return ($this->redis->hSet($this->hash, $key, $value) !== false);
     }
 
@@ -270,7 +318,12 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
     public function set_many(array $keyvaluearray) {
         $pairs = [];
         foreach ($keyvaluearray as $pair) {
-            $pairs[$pair['key']] = $pair['value'];
+            $key = $pair['key'];
+            if ($this->compressor != self::COMPRESSOR_NONE) {
+                $pairs[$key] = $this->compress($pair['value']);
+            } else {
+                $pairs[$key] = $pair['value'];
+            }
         }
         if ($this->redis->hMSet($this->hash, $pairs)) {
             return count($pairs);
@@ -446,7 +499,8 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
             'server' => $data->server,
             'prefix' => $data->prefix,
             'password' => $data->password,
-            'serializer' => $data->serializer
+            'serializer' => $data->serializer,
+            'compressor' => $data->compressor,
         );
     }
 
@@ -465,6 +519,9 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
         if (!empty($config['serializer'])) {
             $data['serializer'] = $config['serializer'];
         }
+        if (!empty($config['compressor'])) {
+            $data['compressor'] = $config['compressor'];
+        }
         $editform->set_data($data);
     }
 
@@ -538,4 +595,115 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
         }
         return $options;
     }
+
+    /**
+     * Gets an array of options to use as the compressor.
+     *
+     * @return array
+     */
+    public static function config_get_compressor_options() {
+        $arr = [
+            self::COMPRESSOR_NONE     => get_string('compressor_none', 'cachestore_redis'),
+            self::COMPRESSOR_PHP_GZIP => get_string('compressor_php_gzip', 'cachestore_redis'),
+        ];
+
+        // Check if the Zstandard PHP extension is installed.
+        if (extension_loaded('zstd')) {
+            $arr[self::COMPRESSOR_PHP_ZSTD] = get_string('compressor_php_zstd', 'cachestore_redis');
+        }
+
+        return $arr;
+    }
+
+    /**
+     * Compress the given value, serializing it first.
+     *
+     * @param mixed $value
+     * @return string
+     */
+    private function compress($value) {
+        $value = $this->serialize($value);
+
+        switch ($this->compressor) {
+            case self::COMPRESSOR_NONE:
+                return $value;
+
+            case self::COMPRESSOR_PHP_GZIP:
+                return gzencode($value);
+
+            case self::COMPRESSOR_PHP_ZSTD:
+                return zstd_compress($value);
+
+            default:
+                debugging("Invalid compressor: {$this->compressor}");
+                return $value;
+        }
+    }
+
+    /**
+     * Uncompresses (deflates) the data, unserialising it afterwards.
+     *
+     * @param string $value
+     * @return mixed
+     */
+    private function uncompress($value) {
+        if ($value === false) {
+            return false;
+        }
+
+        switch ($this->compressor) {
+            case self::COMPRESSOR_NONE:
+                break;
+            case self::COMPRESSOR_PHP_GZIP:
+                $value = gzdecode($value);
+                break;
+            case self::COMPRESSOR_PHP_ZSTD:
+                $value = zstd_uncompress($value);
+                break;
+            default:
+                debugging("Invalid compressor: {$this->compressor}");
+        }
+
+        return $this->unserialize($value);
+    }
+
+    /**
+     * Serializes the data according to the configured serializer.
+     *
+     * @param mixed $value
+     * @return string
+     */
+    private function serialize($value) {
+        switch ($this->serializer) {
+            case Redis::SERIALIZER_NONE:
+                return $value;
+            case Redis::SERIALIZER_PHP:
+                return serialize($value);
+            case defined('Redis::SERIALIZER_IGBINARY') && Redis::SERIALIZER_IGBINARY:
+                return igbinary_serialize($value);
+            default:
+                debugging("Invalid serializer: {$this->serializer}");
+                return $value;
+        }
+    }
+
+    /**
+     * Unserializes the data according to the configured serializer
+     *
+     * @param string $value
+     * @return mixed
+     */
+    private function unserialize($value) {
+        switch ($this->serializer) {
+            case Redis::SERIALIZER_NONE:
+                return $value;
+            case Redis::SERIALIZER_PHP:
+                return unserialize($value);
+            case defined('Redis::SERIALIZER_IGBINARY') && Redis::SERIALIZER_IGBINARY:
+                return igbinary_unserialize($value);
+            default:
+                debugging("Invalid serializer: {$this->serializer}");
+                return $value;
+        }
+    }
 }
diff --git a/cache/stores/redis/tests/compressor_test.php b/cache/stores/redis/tests/compressor_test.php
new file mode 100644 (file)
index 0000000..0d9a583
--- /dev/null
@@ -0,0 +1,287 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Redis cache test.
+ *
+ * If you wish to use these unit tests all you need to do is add the following definition to
+ * your config.php file.
+ *
+ * define('TEST_CACHESTORE_REDIS_TESTSERVERS', '127.0.0.1');
+ *
+ * @package   cachestore_redis
+ * @copyright 2018 Catalyst IT Australia {@link http://www.catalyst-au.net}
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+require_once(__DIR__.'/../../../tests/fixtures/stores.php');
+require_once(__DIR__.'/../lib.php');
+
+/**
+ * Redis cache test - compressor settings.
+ *
+ * @package   cachestore_redis
+ * @author    Daniel Thee Roperto <daniel.roperto@catalyst-au.net>
+ * @copyright 2018 Catalyst IT Australia {@link http://www.catalyst-au.net}
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class cachestore_redis_compressor_test extends advanced_testcase {
+
+    /**
+     * Test set up
+     */
+    public function setUp() {
+        if (!cachestore_redis::are_requirements_met() || !defined('TEST_CACHESTORE_REDIS_TESTSERVERS')) {
+            $this->markTestSkipped('Could not test cachestore_redis. Requirements are not met.');
+        }
+
+        parent::setUp();
+    }
+
+    /**
+     * Create a cachestore.
+     *
+     * @param int $compressor
+     * @param int $serializer
+     * @return cachestore_redis
+     */
+    public function create_store($compressor, $serializer) {
+        /** @var cache_definition $definition */
+        $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_redis', 'phpunit_test');
+        $config = cachestore_redis::unit_test_configuration();
+        $config['compressor'] = $compressor;
+        $config['serializer'] = $serializer;
+        $store = new cachestore_redis('Test', $config);
+        $store->initialise($definition);
+
+        return $store;
+    }
+
+    /**
+     * It misses a value.
+     */
+    public function test_it_can_miss_one() {
+        $store = $this->create_store(cachestore_redis::COMPRESSOR_PHP_GZIP, Redis::SERIALIZER_PHP);
+
+        self::assertFalse($store->get('missme'));
+    }
+
+    /**
+     * It misses many values.
+     */
+    public function test_it_can_miss_many() {
+        $store = $this->create_store(cachestore_redis::COMPRESSOR_PHP_GZIP, Redis::SERIALIZER_PHP);
+
+        $expected = ['missme' => false, 'missmetoo' => false];
+        $actual = $store->get_many(array_keys($expected));
+        self::assertSame($expected, $actual);
+    }
+
+    /**
+     * It misses some values.
+     */
+    public function test_it_can_miss_some() {
+        $store = $this->create_store(cachestore_redis::COMPRESSOR_PHP_GZIP, Redis::SERIALIZER_PHP);
+        $store->set('iamhere', 'youfoundme');
+
+        $expected = ['missme' => false, 'missmetoo' => false, 'iamhere' => 'youfoundme'];
+        $actual = $store->get_many(array_keys($expected));
+        self::assertSame($expected, $actual);
+    }
+
+    /**
+     * A provider for test_works_with_different_types
+     *
+     * @return array
+     */
+    public function provider_for_test_it_works_with_different_types() {
+        $object = new stdClass();
+        $object->field = 'value';
+
+        return [
+            ['string', 'Abc Def'],
+            ['string_empty', ''],
+            ['string_binary', gzencode('some binary data')],
+            ['int', 123],
+            ['int_zero', 0],
+            ['int_negative', -100],
+            ['int_huge', PHP_INT_MAX],
+            ['float', 3.14],
+            ['boolean_true', true],
+            // Boolean 'false' is not tested as it is not allowed in Moodle.
+            ['array', [1, 'b', 3.4]],
+            ['array_map', ['a' => 'b', 'c' => 'd']],
+            ['object_stdClass', $object],
+            ['null', null],
+        ];
+    }
+
+    /**
+     * It works with different types.
+     *
+     * @dataProvider provider_for_test_it_works_with_different_types
+     * @param string $key
+     * @param mixed $value
+     */
+    public function test_it_works_with_different_types($key, $value) {
+        $store = $this->create_store(cachestore_redis::COMPRESSOR_PHP_GZIP, Redis::SERIALIZER_PHP);
+        $store->set($key, $value);
+
+        self::assertEquals($value, $store->get($key), "Failed set/get for: {$key}");
+    }
+
+    /**
+     * Test it works with different types for many.
+     */
+    public function test_it_works_with_different_types_for_many() {
+        $store = $this->create_store(cachestore_redis::COMPRESSOR_PHP_GZIP, Redis::SERIALIZER_PHP);
+
+        $provider = $this->provider_for_test_it_works_with_different_types();
+        $keys = [];
+        $values = [];
+        $expected = [];
+        foreach ($provider as $item) {
+            $keys[] = $item[0];
+            $values[] = ['key' => $item[0], 'value' => $item[1]];
+            $expected[$item[0]] = $item[1];
+        }
+        $store->set_many($values);
+        $actual = $store->get_many($keys);
+        self::assertEquals($expected, $actual);
+    }
+
+    /**
+     * Provider for set/get combination tests.
+     *
+     * @return array
+     */
+    public function provider_for_tests_setget() {
+        $data = [
+            ['none, none',
+                Redis::SERIALIZER_NONE, cachestore_redis::COMPRESSOR_NONE,
+                'value1', 'value2'],
+            ['none, gzip',
+                Redis::SERIALIZER_NONE, cachestore_redis::COMPRESSOR_PHP_GZIP,
+                gzencode('value1'), gzencode('value2')],
+            ['php, none',
+                Redis::SERIALIZER_PHP, cachestore_redis::COMPRESSOR_NONE,
+                serialize('value1'), serialize('value2')],
+            ['php, gzip',
+                Redis::SERIALIZER_PHP, cachestore_redis::COMPRESSOR_PHP_GZIP,
+                gzencode(serialize('value1')), gzencode(serialize('value2'))],
+        ];
+
+        if (defined('Redis::SERIALIZER_IGBINARY')) {
+            $data[] = [
+                'igbinary, none',
+                    Redis::SERIALIZER_IGBINARY, cachestore_redis::COMPRESSOR_NONE,
+                    igbinary_serialize('value1'), igbinary_serialize('value2'),
+            ];
+            $data[] = [
+                'igbinary, gzip',
+                    Redis::SERIALIZER_IGBINARY, cachestore_redis::COMPRESSOR_PHP_GZIP,
+                    gzencode(igbinary_serialize('value1')), gzencode(igbinary_serialize('value2')),
+            ];
+        }
+
+        if (extension_loaded('zstd')) {
+            $data[] = [
+                'none, zstd',
+                Redis::SERIALIZER_NONE, cachestore_redis::COMPRESSOR_PHP_ZSTD,
+                zstd_compress('value1'), zstd_compress('value2'),
+            ];
+            $data[] = [
+                'php, zstd',
+                Redis::SERIALIZER_PHP, cachestore_redis::COMPRESSOR_PHP_ZSTD,
+                zstd_compress(serialize('value1')), zstd_compress(serialize('value2')),
+            ];
+
+            if (defined('Redis::SERIALIZER_IGBINARY')) {
+                $data[] = [
+                    'igbinary, zstd',
+                    Redis::SERIALIZER_IGBINARY, cachestore_redis::COMPRESSOR_PHP_ZSTD,
+                    zstd_compress(igbinary_serialize('value1')), zstd_compress(igbinary_serialize('value2')),
+                ];
+            }
+        }
+
+        return $data;
+    }
+
+    /**
+     * Test we can use get and set with all combinations.
+     *
+     * @dataProvider provider_for_tests_setget
+     * @param string $name
+     * @param int $serializer
+     * @param int $compressor
+     * @param string $rawexpected1
+     * @param string $rawexpected2
+     */
+    public function test_it_can_use_getset($name, $serializer, $compressor, $rawexpected1, $rawexpected2) {
+        // Create a connection with the desired serialisation.
+        $store = $this->create_store($compressor, $serializer);
+        $store->set('key', 'value1');
+
+        // Disable compressor and serializer to check the actual stored value.
+        $rawstore = $this->create_store(cachestore_redis::COMPRESSOR_NONE, Redis::SERIALIZER_NONE);
+
+        $data = $store->get('key');
+        $rawdata = $rawstore->get('key');
+        self::assertSame('value1', $data, "Invalid serialisation/unserialisation for: {$name}");
+        self::assertSame($rawexpected1, $rawdata, "Invalid rawdata for: {$name}");
+    }
+
+    /**
+     * Test we can use get and set many with all combinations.
+     *
+     * @dataProvider provider_for_tests_setget
+     * @param string $name
+     * @param int $serializer
+     * @param int $compressor
+     * @param string $rawexpected1
+     * @param string $rawexpected2
+     */
+    public function test_it_can_use_getsetmany($name, $serializer, $compressor, $rawexpected1, $rawexpected2) {
+        $many = [
+            ['key' => 'key1', 'value' => 'value1'],
+            ['key' => 'key2', 'value' => 'value2'],
+        ];
+        $keys = ['key1', 'key2'];
+        $expectations = ['key1' => 'value1', 'key2' => 'value2'];
+        $rawexpectations = ['key1' => $rawexpected1, 'key2' => $rawexpected2];
+
+        // Create a connection with the desired serialisation.
+        $store = $this->create_store($compressor, $serializer);
+        $store->set_many($many);
+
+        // Disable compressor and serializer to check the actual stored value.
+        $rawstore = $this->create_store(cachestore_redis::COMPRESSOR_NONE, Redis::SERIALIZER_NONE);
+
+        $data = $store->get_many($keys);
+        $rawdata = $rawstore->get_many($keys);
+        foreach ($keys as $key) {
+            self::assertSame($expectations[$key],
+                             $data[$key],
+                             "Invalid serialisation/unserialisation for {$key} with serializer {$name}");
+            self::assertSame($rawexpectations[$key],
+                             $rawdata[$key],
+                             "Invalid rawdata for {$key} with serializer {$name}");
+        }
+    }
+}
index 4a801da..eaf6344 100644 (file)
@@ -1,6 +1,9 @@
 This files describes API changes in /cache/stores/* - cache store plugins.
 Information provided here is intended especially for developers.
 
+=== 3.8 ===
+* The Redis cache store can now make use of the Zstandard compression algorithm (see MDL-66428).
+
 === 3.7 ===
 * Upgraded MongoDB cache store to use the new lower level PHP-driver and MongoDB PHP Library.
 * The mongodb extension has replaced the old mongo extension. The mongodb pecl extension >= 1.5 must be installed to use MongoDB
index 45d50d1..2aa96e1 100644 (file)
@@ -37,6 +37,14 @@ use \core_customfield\category_controller;
  */
 class core_customfield_api_testcase extends advanced_testcase {
 
+    /**
+     * This method is called after the last test of this test class is run.
+     */
+    public static function tearDownAfterClass() {
+        $handler = core_course\customfield\course_handler::create();
+        $handler->delete_all();
+    }
+
     /**
      * Tests set up.
      */
index 656bc5b..56aa2d0 100644 (file)
@@ -36,6 +36,14 @@ use \core_customfield\field_controller;
  */
 class core_customfield_category_controller_testcase extends advanced_testcase {
 
+    /**
+     * This method is called after the last test of this test class is run.
+     */
+    public static function tearDownAfterClass() {
+        $handler = core_course\customfield\course_handler::create();
+        $handler->delete_all();
+    }
+
     /**
      * Tests set up.
      */
index ac5b405..7cf0b7e 100644 (file)
@@ -34,6 +34,14 @@ use core_customfield\data_controller;
  */
 class core_customfield_data_controller_testcase extends advanced_testcase {
 
+    /**
+     * This method is called after the last test of this test class is run.
+     */
+    public static function tearDownAfterClass() {
+        $handler = core_course\customfield\course_handler::create();
+        $handler->delete_all();
+    }
+
     /**
      * Tests set up.
      */
index b704469..e490ef1 100644 (file)
@@ -38,6 +38,14 @@ use \core_customfield\field_controller;
  */
 class core_customfield_field_controller_testcase extends advanced_testcase {
 
+    /**
+     * This method is called after the last test of this test class is run.
+     */
+    public static function tearDownAfterClass() {
+        $handler = core_course\customfield\course_handler::create();
+        $handler->delete_all();
+    }
+
     /**
      * Tests set up.
      */
index 4788d9c..059c868 100644 (file)
@@ -35,6 +35,14 @@ defined('MOODLE_INTERNAL') || die();
  */
 class core_customfield_generator_testcase extends advanced_testcase {
 
+    /**
+     * This method is called after the last test of this test class is run.
+     */
+    public static function tearDownAfterClass() {
+        $handler = core_course\customfield\course_handler::create();
+        $handler->delete_all();
+    }
+
     /**
      * Get generator
      * @return core_customfield_generator
index 9a15ced..69ea102 100644 (file)
@@ -45,6 +45,14 @@ class core_customfield_privacy_testcase extends provider_testcase {
     /** @var \core_customfield\field_controller[] */
     private $cffields = [];
 
+    /**
+     * This method is called after the last test of this test class is run.
+     */
+    public static function tearDownAfterClass() {
+        $handler = core_course\customfield\course_handler::create();
+        $handler->delete_all();
+    }
+
     /**
      * Set up
      */
index e44cef6..9604b19 100644 (file)
@@ -58,14 +58,17 @@ class core_enrollib_testcase extends advanced_testcase {
 
         $course1 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'Z',
+            'idnumber' => '123',
             'category' => $category1->id,
         ));
         $course2 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'X',
+            'idnumber' => '789',
             'category' => $category2->id,
         ));
         $course3 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'Y',
+            'idnumber' => '456',
             'category' => $category2->id,
             'visible' => 0,
         ));
@@ -163,7 +166,7 @@ class core_enrollib_testcase extends advanced_testcase {
         $this->assertTrue(property_exists($course, 'timecreated'));
 
         $courses = enrol_get_all_users_courses($user2->id, false, null, 'id DESC');
-        $this->assertEquals(array($course3->id, $course2->id, $course1->id), array_keys($courses));
+        $this->assertEquals(array($course2->id, $course3->id, $course1->id), array_keys($courses));
 
         // Make sure that implicit sorting defined in navsortmycoursessort is respected.
 
@@ -175,7 +178,54 @@ class core_enrollib_testcase extends advanced_testcase {
         // But still the explicit sorting takes precedence over the implicit one.
 
         $courses = enrol_get_all_users_courses($user1->id, false, null, 'shortname DESC');
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
+
+        // Make sure that implicit visibility sorting defined in navsortmycourseshiddenlast is respected for all course sortings.
+
+        $CFG->navsortmycoursessort = 'sortorder';
+        $CFG->navsortmycourseshiddenlast = true;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'sortorder';
+        $CFG->navsortmycourseshiddenlast = false;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course1->id, $course3->id, $course2->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'fullname';
+        $CFG->navsortmycourseshiddenlast = true;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'fullname';
+        $CFG->navsortmycourseshiddenlast = false;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course1->id, $course2->id, $course3->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'shortname';
+        $CFG->navsortmycourseshiddenlast = true;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course3->id, $course1->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'shortname';
+        $CFG->navsortmycourseshiddenlast = false;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course3->id, $course1->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'idnumber';
+        $CFG->navsortmycourseshiddenlast = true;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'idnumber';
+        $CFG->navsortmycourseshiddenlast = false;
+        $courses = enrol_get_all_users_courses($user1->id);
         $this->assertEquals(array($course1->id, $course3->id, $course2->id), array_keys($courses));
+
+        // But still the explicit visibility sorting takes precedence over the implicit one.
+
+        $courses = enrol_get_all_users_courses($user1->id, false, null, 'visible DESC, shortname DESC');
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
     }
 
     public function test_enrol_user_sees_own_courses() {
index 623e935..27bc411 100644 (file)
@@ -88,7 +88,7 @@ $string['welcomep20'] = 'Je krijgt deze pagina te zien omdat je met succes het <
 $string['welcomep30'] = 'Deze uitgave van <strong>{$a->installername}</strong> bevat de software die nodig is om een omgeving te creĆ«ren waarin <strong>Moodle</strong> zal werken, namelijk:';
 $string['welcomep40'] = 'Dit pakket bevat ook <strong>Moodle {$a->moodlerelease} ({$a->moodleversion})</strong>.';
 $string['welcomep50'] = 'Het gebruik van alle programma\'s in dit pakket wordt geregeld door hun respectievelijke licenties. Het complete <strong>{$a->installername}</strong> pakket is
-<a href="http://www.opensource.org/docs/definition_plain.html">open source</a> en wordt verdeeld onder de <a href="http://www.gnu.org/copyleft/gpl.html">GPL</a> licentie.';
+<a href="https://www.opensource.org/docs/definition_plain.html">open source</a> en wordt verdeeld onder de <a href="https://www.gnu.org/copyleft/gpl.html">GPL</a> licentie.';
 $string['welcomep60'] = 'De volgende pagina\'s leiden je door een aantal makkelijk te volgen stappen om <strong>Moodle</strong> te installeren op je computer. Je kunt de standaardinstellingen overnemen of, optioneel, ze aanpassen aan je noden.';
 $string['welcomep70'] = 'Klik op de "volgende"-knop om verder te gaan met de installatie van <strong>Moodle</strong>';
 $string['wwwroot'] = 'Web adres';
index 908b8cd..ea89b1d 100644 (file)
@@ -822,6 +822,8 @@ $string['navshowallcourses'] = 'Show all courses';
 $string['navshowcategories'] = 'Show course categories';
 $string['navshowmycoursecategories'] = 'Show my course categories';
 $string['navshowmycoursecategories_help'] = 'If enabled courses in the users my courses branch will be shown in categories.';
+$string['navsortmycourseshiddenlast'] = 'Sort my hidden courses last';
+$string['navsortmycourseshiddenlast_help'] = 'If enabled, any hidden courses will be listed after visible courses (for users who can view hidden courses). Otherwise, all courses, regardless of their visibility, will be listed according to the \'Sort my courses\' setting.';
 $string['navsortmycoursessort'] = 'Sort my courses';
 $string['navsortmycoursessort_help'] = 'This determines whether courses are listed under My courses according to the sort order (i.e. the order set in Site administration > Courses > Manage courses and categories) or alphabetically by course setting.';
 $string['never'] = 'Never';
index 71a17ac..7f8979d 100644 (file)
@@ -88,7 +88,8 @@ class api {
 
         $curl = new curl();
         $serverurl = HUB_MOODLEORGHUBURL . "/local/hub/webservice/webservices.php";
-        $curloutput = @json_decode($curl->get($serverurl, $params), true);
+        $query = http_build_query($params, '', '&');
+        $curloutput = @json_decode($curl->post($serverurl, $query), true);
         $info = $curl->get_info();
         if ($curl->get_errno()) {
             // Connection error.
index 25fc0d4..cd716ef 100644 (file)
@@ -40,11 +40,11 @@ use html_writer;
  */
 class registration {
 
-    /** @var Fields used in a site registration form.
+    /** @var array Fields used in a site registration form.
      * IMPORTANT: any new fields with non-empty defaults have to be added to CONFIRM_NEW_FIELDS */
-    const FORM_FIELDS = ['name', 'description', 'contactname', 'contactemail', 'contactphone', 'imageurl', 'privacy', 'street',
-        'regioncode', 'countrycode', 'geolocation', 'contactable', 'emailalert', 'emailalertemail', 'commnews', 'commnewsemail',
-        'language', 'policyagreed'];
+    const FORM_FIELDS = ['policyagreed', 'language', 'countrycode', 'privacy',
+        'contactemail', 'contactable', 'emailalert', 'emailalertemail', 'commnews', 'commnewsemail',
+        'contactname', 'name', 'description', 'imageurl', 'contactphone', 'regioncode', 'geolocation', 'street'];
 
     /** @var List of new FORM_FIELDS or siteinfo fields added indexed by the version when they were added.
      * If site was already registered, admin will be promted to confirm new registration data manually. Until registration is manually confirmed,
@@ -348,6 +348,13 @@ class registration {
         $record['timemodified'] = time();
         $DB->update_record('registration_hubs', $record);
         self::$registration = null;
+
+        $siteinfo = self::get_site_info();
+        if (strlen(http_build_query($siteinfo)) > 1800) {
+            // Update registration again because the initial request was too long and could have been truncated.
+            api::update_registration($siteinfo);
+            self::$registration = null;
+        }
     }
 
     /**
@@ -396,10 +403,21 @@ class registration {
         }
 
         $params = self::get_site_info();
-        $params['token'] = $hub->token;
+
+        // The most conservative limit for the redirect URL length is 2000 characters. Only pass parameters before
+        // we reach this limit. The next registration update will update all fields.
+        // We will also update registration after we receive confirmation from moodle.net.
+        $url = new moodle_url(HUB_MOODLEORGHUBURL . '/local/hub/siteregistration.php',
+            ['token' => $hub->token, 'url' => $params['url']]);
+        foreach ($params as $key => $value) {
+            if (strlen($url->out(false, [$key => $value])) > 2000) {
+                break;
+            }
+            $url->param($key, $value);
+        }
 
         $SESSION->registrationredirect = $returnurl;
-        redirect(new moodle_url(HUB_MOODLEORGHUBURL . '/local/hub/siteregistration.php', $params));
+        redirect($url);
     }
 
     /**
index 7a09fa5..f58f9ef 100644 (file)
@@ -75,7 +75,7 @@ class site_registration_form extends \moodleform {
         $mform->addElement('header', 'moodle', get_string('registrationinfo', 'hub'));
 
         $mform->addElement('text', 'name', get_string('sitename', 'hub'),
-            array('class' => 'registration_textfield'));
+            array('class' => 'registration_textfield', 'maxlength' => 255));
         $mform->setType('name', PARAM_TEXT);
         $mform->addHelpButton('name', 'sitename', 'hub');
 
index 425deab..37a3784 100644 (file)
@@ -3556,7 +3556,7 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2019092700.01);
     }
 
-    if ($oldversion < 2019100400.01) {
+    if ($oldversion < 2019100800.02) {
         // Rename the official moodle sites directory the site is registered with.
         $DB->execute("UPDATE {registration_hubs}
                          SET hubname = ?, huburl = ?
@@ -3571,7 +3571,7 @@ function xmldb_main_upgrade($oldversion) {
             }
         }
 
-        upgrade_main_savepoint(true, 2019100400.01);
+        upgrade_main_savepoint(true, 2019100800.02);
     }
 
     return true;
index aac1cac..fbb5c39 100644 (file)
@@ -567,13 +567,8 @@ function enrol_get_my_courses($fields = null, $sort = null, $limit = 0, $coursei
     $offset = 0, $excludecourses = []) {
     global $DB, $USER, $CFG;
 
-    if ($sort === null) {
-        if (empty($CFG->navsortmycoursessort)) {
-            $sort = 'visible DESC, sortorder ASC';
-        } else {
-            $sort = 'visible DESC, '.$CFG->navsortmycoursessort.' ASC';
-        }
-    }
+    // Re-Arrange the course sorting according to the admin settings.
+    $sort = enrol_get_courses_sortingsql($sort);
 
     // Guest account does not have any enrolled courses.
     if (!$allaccessible && (isguestuser() or !isloggedin())) {
@@ -804,6 +799,41 @@ function enrol_get_course_info_icons($course, array $instances = NULL) {
     return $icons;
 }
 
+/**
+ * Returns SQL ORDER arguments which reflect the admin settings to sort my courses.
+ *
+ * @param string|null $sort SQL ORDER arguments which were originally requested (optionally).
+ * @return string SQL ORDER arguments.
+ */
+function enrol_get_courses_sortingsql($sort = null) {
+    global $CFG;
+
+    // Prepare the visible SQL fragment as empty.
+    $visible = '';
+    // Only create a visible SQL fragment if the caller didn't already pass a sort order which contains the visible field.
+    if ($sort === null || strpos($sort, 'visible') === false) {
+        // If the admin did not explicitly want to have shown and hidden courses sorted as one list, we will sort hidden
+        // courses to the end of the course list.
+        if (!isset($CFG->navsortmycourseshiddenlast) || $CFG->navsortmycourseshiddenlast == true) {
+            $visible = 'visible DESC, ';
+        }
+    }
+
+    // Only create a sortorder SQL fragment if the caller didn't already pass one.
+    if ($sort === null) {
+        // If the admin has configured a course sort order, we will use this.
+        if (!empty($CFG->navsortmycoursessort)) {
+            $sort = $CFG->navsortmycoursessort . ' ASC';
+
+            // Otherwise we will fall back to the sortorder sorting.
+        } else {
+            $sort = 'sortorder ASC';
+        }
+    }
+
+    return $visible . $sort;
+}
+
 /**
  * Returns course enrolment detailed information.
  *
@@ -955,15 +985,10 @@ function enrol_user_sees_own_courses($user = null) {
  * @return array
  */
 function enrol_get_all_users_courses($userid, $onlyactive = false, $fields = null, $sort = null) {
-    global $CFG, $DB;
+    global $DB;
 
-    if ($sort === null) {
-        if (empty($CFG->navsortmycoursessort)) {
-            $sort = 'visible DESC, sortorder ASC';
-        } else {
-            $sort = 'visible DESC, '.$CFG->navsortmycoursessort.' ASC';
-        }
-    }
+    // Re-Arrange the course sorting according to the admin settings.
+    $sort = enrol_get_courses_sortingsql($sort);
 
     // Guest account does not have any courses
     if (isguestuser($userid) or empty($userid)) {
index 1dcb34f..1400b4c 100644 (file)
@@ -9,7 +9,8 @@
                    {{#error}}is-invalid{{/error}}"
         name="{{element.name}}"
         id="{{element.id}}"
-        {{#element.multiple}}multiple size="{{element.size}}"{{/element.multiple}}
+        {{#element.multiple}}multiple{{/element.multiple}}
+        {{#element.size}}size="{{element.size}}"{{/element.size}}
         {{#error}}
             autofocus aria-describedby="{{element.iderror}}"
         {{/error}}
index f701053..1d320b3 100644 (file)
                        {{#error}}is-invalid{{/error}}"
             name="{{element.name}}"
             id="{{element.id}}"
-            {{#element.multiple}}multiple size="{{element.size}}"{{/element.multiple}}
+            {{#element.multiple}}multiple{{/element.multiple}}
+            {{#element.size}}size="{{element.size}}"{{/element.size}}
             {{#error}}
                 autofocus aria-describedby="{{element.iderror}}"
             {{/error}}
index 0164335..84b449d 100644 (file)
@@ -4,6 +4,7 @@
     <select class="form-control custom-select {{#error}}is-invalid{{/error}}" name="{{element.name}}"
         id="{{element.id}}"
         {{#element.multiple}}multiple{{/element.multiple}}
+        {{#element.size}}size="{{element.size}}"{{/element.size}}
         {{#error}}
             autofocus aria-describedby="{{element.iderror}}"
         {{/error}}
index 15738f7..987e996 100644 (file)
@@ -4,6 +4,7 @@
         <select class="form-control custom-select {{#error}}is-invalid{{/error}}" name="{{element.name}}"
             id="{{element.id}}"
             {{#element.multiple}}multiple{{/element.multiple}}
+            {{#element.size}}size="{{element.size}}"{{/element.size}}
             {{#error}}
                 autofocus aria-describedby="{{element.iderror}}"
             {{/error}}
index b49e0a5..28c5874 100644 (file)
@@ -4,6 +4,7 @@
         <select class="custom-select {{#error}}is-invalid{{/error}}" name="{{element.name}}"
             id="{{element.id}}"
             {{#element.multiple}}multiple{{/element.multiple}}
+            {{#element.size}}size="{{element.size}}"{{/element.size}}
             {{#error}}
                 autofocus aria-describedby="{{element.iderror}}"
             {{/error}}
index 8f87320..cd8a9d5 100644 (file)
 >
     <div class="position-relative h-100" data-region="content-container" style="overflow-y: auto; overflow-x: hidden">
         <div class="content-message-container hidden h-100 px-2 pt-0" data-region="content-message-container" role="log" style="overflow-y: auto; overflow-x: hidden">
-            <div class="py-3 bg-light sticky-top z-index-1 border-bottom text-center hidden" data-region="contact-request-sent-message-container">
+            <div class="py-3 sticky-top z-index-1 border-bottom text-center hidden" data-region="contact-request-sent-message-container">
                 <p class="m-0">{{#str}} contactrequestsent, core_message {{/str}}</p>
                 <p class="font-italic font-weight-light" data-region="text"></p>
             </div>
-            <div class="p-3 bg-light text-center hidden" data-region="self-conversation-message-container">
+            <div class="p-3 text-center hidden" data-region="self-conversation-message-container">
                 <p class="m-0">{{#str}} selfconversation, core_message {{/str}}</p>
                 <p class="font-italic font-weight-light" data-region="text">{{#str}} selfconversationdefaultmessage, core_message {{/str}}</p>
            </div>
index 0205195..0e7d128 100644 (file)
@@ -1925,8 +1925,12 @@ class quiz_attempt {
     }
 
     /**
-     * Replace a question in an attempt with a new attempt at the same qestion.
-     * @param int $slot the questoin to restart.
+     * Replace a question in an attempt with a new attempt at the same question.
+     *
+     * Well, for randomised questions, it won't be the same question, it will be
+     * a different randomised selection.
+     *
+     * @param int $slot the question to restart.
      * @param int $timestamp the timestamp to record for this action.
      */
     public function process_redo_question($slot, $timestamp) {
@@ -1938,7 +1942,7 @@ class quiz_attempt {
         }
 
         $qubaids = new \mod_quiz\question\qubaids_for_users_attempts(
-                $this->get_quizid(), $this->get_userid());
+                $this->get_quizid(), $this->get_userid(), 'all', true);
 
         $transaction = $DB->start_delegated_transaction();
 
index fbadb44..4398197 100644 (file)
@@ -128,11 +128,16 @@ Feature: Allow students to redo questions in a practice quiz, without starting a
     And quiz "Quiz 2" contains the following questions:
       | question                | page |
       | Random (Test questions) | 1    |
+    And user "student" has started an attempt at quiz "Quiz 2" randomised as follows:
+      | slot | actualquestion |
+      | 1    | TF1            |
     And I log in as "student"
     And I am on "Course 1" course homepage
     When I follow "Quiz 2"
-    And I press "Attempt quiz now"
+    And I press "Continue the last attempt"
+    And I should see "First question"
     And I click on "False" "radio"
     And I click on "Check" "button"
     And I press "Try another question like this one"
-    Then "Check" "button" should exist
+    Then I should see "Second question"
+    And "Check" "button" should exist
index 99fb72f..eaa8b44 100644 (file)
@@ -34,6 +34,37 @@ Feature: Backup and restore of quizzes
     Then I should see "TF1"
     And I should see "TF2"
 
+  @javascript
+  Scenario: Backup and restore a course containing a quiz with user data.
+    Given the following "activities" exist:
+      | activity   | name   | intro              | course | idnumber |
+      | quiz       | Quiz 1 | For testing backup | C1     | quiz1    |
+    And the following "questions" exist:
+      | questioncategory | qtype       | name | questiontext    |
+      | Test questions   | truefalse   | TF1  | First question  |
+      | Test questions   | truefalse   | TF2  | Second question |
+    And quiz "Quiz 1" contains the following questions:
+      | question | page |
+      | TF1      | 1    |
+      | TF2      | 2    |
+    And the following "users" exist:
+      | username |
+      | student  |
+    And the following "course enrolments" exist:
+      | user    | course | role    |
+      | student | C1     | student |
+    And user "student" has attempted "Quiz 1" with responses:
+      | slot | response |
+      | 1    | True     |
+      | 2    | False    |
+    When I backup "Course 1" course using this options:
+      | Confirmation | Filename | test_backup.mbz |
+    And I restore "test_backup.mbz" backup into a new course using this options:
+      | Schema | Course name | Restored course |
+    Then I should see "Restored course"
+    And I follow "Quiz 1"
+    And I should see "Attempts: 1"
+
   @javascript @_file_upload
   Scenario: Restore a Moodle 2.8 quiz backup
     When I am on "Course 1" course homepage
index cfa4e45..0269b75 100644 (file)
@@ -631,7 +631,8 @@ class behat_mod_quiz extends behat_question_base {
      *                random questions. If so, this will let you control which actual
      *                question gets picked when this slot is 'randomised' at the
      *                start of the attempt. If you don't specify, then one will be picked
-     *                at random (which might make the reponse meaningless).
+     *                at random (which might make the response meaningless).
+     *                Give the question name.
      * variant        This column is similar, and also options. It is only needed if
      *                the question that ends up in this slot returns something greater
      *                than 1 for $question->get_num_variants(). Like with actualquestion,
@@ -708,7 +709,7 @@ class behat_mod_quiz extends behat_question_base {
      * @param string $username the username of the user that will attempt.
      * @param string $quizname the name of the quiz the user will attempt.
      * @param TableNode $attemptinfo information about the questions to add, as above.
-     * @Given /^user "([^"]*)" has started an attempt at quiz "([^"]*) randomised as follows:"$/
+     * @Given /^user "([^"]*)" has started an attempt at quiz "([^"]*)" randomised as follows:$/
      */
     public function user_has_started_an_attempt_at_quiz_with_details($username, $quizname, TableNode $attemptinfo) {
         global $DB;
index 7d5f180..2befa39 100644 (file)
@@ -25,9 +25,6 @@
  * The exception to this is some of the reporting methods, like
  * {@link question_engine_data_mapper::load_attempts_at_question()}.
  *
- * (TODO, probably we should split this class up, so that it has no public
- * methods. They should all be moved to a new public class.)
- *
  * A note for future reference. This code is pretty efficient but there are some
  * potential optimisations that could be contemplated, at the cost of making the
  * code more complex:
@@ -249,7 +246,7 @@ class question_engine_data_mapper {
      *
      * Private method, only for use by other parts of the question engine.
      *
-     * @param question_attempt_step $qa the step to store.
+     * @param question_attempt_step $step the step to store.
      * @param int $questionattemptid the question attept id this step belongs to.
      * @param int $seq the sequence number of this stop.
      * @param context $context the context of the owning question_usage_by_activity.
@@ -324,7 +321,7 @@ class question_engine_data_mapper {
      * Private method, only for use by other parts of the question engine.
      *
      * @param int $stepid the id of the step to load.
-     * @param question_attempt_step the step that was loaded.
+     * @return question_attempt_step the step that was loaded.
      */
     public function load_question_attempt_step($stepid) {
         $records = $this->db->get_recordset_sql("
@@ -370,7 +367,7 @@ WHERE
      * wish to load one qa, in which case you may call this method.
      *
      * @param int $questionattemptid the id of the question attempt to load.
-     * @param question_attempt the question attempt that was loaded.
+     * @return question_attempt the question attempt that was loaded.
      */
     public function load_question_attempt($questionattemptid) {
         $records = $this->db->get_recordset_sql("
@@ -432,7 +429,7 @@ ORDER BY
      * rather than calling this method directly.
      *
      * @param int $qubaid the id of the usage to load.
-     * @param question_usage_by_activity the usage that was loaded.
+     * @return question_usage_by_activity the usage that was loaded.
      */
     public function load_questions_usage_by_activity($qubaid) {
         $records = $this->db->get_recordset_sql("
@@ -558,12 +555,18 @@ ORDER BY
      *
      * @param qubaid_condition $qubaids used to restrict which usages are included
      *                                  in the query. See {@link qubaid_condition}.
-     * @param array            $slots   A list of slots for the questions you want to know about.
+     * @param array|null       $slots   (optional) list of slots for which to return information. Default all slots.
      * @param string|null      $fields
      * @return array of records. See the SQL in this function to see the fields available.
      */
-    public function load_questions_usages_latest_steps(qubaid_condition $qubaids, $slots, $fields = null) {
-        list($slottest, $params) = $this->db->get_in_or_equal($slots, SQL_PARAMS_NAMED, 'slot');
+    public function load_questions_usages_latest_steps(qubaid_condition $qubaids, $slots = null, $fields = null) {
+        if ($slots !== null) {
+            [$slottest, $params] = $this->db->get_in_or_equal($slots, SQL_PARAMS_NAMED, 'slot');
+            $slotwhere = " AND qa.slot {$slottest}";
+        } else {
+            $slotwhere = '';
+            $params = [];
+        }
 
         if ($fields === null) {
             $fields = "qas.id,
@@ -599,8 +602,8 @@ JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id
         AND qas.sequencenumber = {$this->latest_step_for_qa_subquery()}
 
 WHERE
-    {$qubaids->where()} AND
-    qa.slot $slottest
+    {$qubaids->where()}
+    $slotwhere
         ", $params + $qubaids->from_where_params());
 
         return $records;
@@ -615,14 +618,19 @@ WHERE
      *
      * @param qubaid_condition $qubaids used to restrict which usages are included
      * in the query. See {@link qubaid_condition}.
-     * @param array $slots A list of slots for the questions you want to konw about.
-     * @return array The array keys are slot,qestionid. The values are objects with
+     * @param array|null $slots (optional) list of slots for which to return information. Default all slots.
+     * @return array The array keys are 'slot,questionid'. The values are objects with
      * fields $slot, $questionid, $inprogress, $name, $needsgrading, $autograded,
      * $manuallygraded and $all.
      */
-    public function load_questions_usages_question_state_summary(
-            qubaid_condition $qubaids, $slots) {
-        list($slottest, $params) = $this->db->get_in_or_equal($slots, SQL_PARAMS_NAMED, 'slot');
+    public function load_questions_usages_question_state_summary(qubaid_condition $qubaids, $slots = null) {
+        if ($slots !== null) {
+            [$slottest, $params] = $this->db->get_in_or_equal($slots, SQL_PARAMS_NAMED, 'slot');
+            $slotwhere = " AND qa.slot {$slottest}";
+        } else {
+            $slotwhere = '';
+            $params = [];
+        }
 
         $rs = $this->db->get_recordset_sql("
 SELECT
@@ -640,8 +648,8 @@ JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id
 JOIN {question} q ON q.id = qa.questionid
 
 WHERE
-    {$qubaids->where()} AND
-    qa.slot $slottest
+    {$qubaids->where()}
+    $slotwhere
 
 GROUP BY
     qa.slot,
@@ -697,7 +705,7 @@ ORDER BY
      *
      * @param qubaid_condition $qubaids used to restrict which usages are included
      * in the query. See {@link qubaid_condition}.
-     * @param int $slot The slot for the questions you want to konw about.
+     * @param int $slot The slot for the questions you want to know about.
      * @param int $questionid (optional) Only return attempts that were of this specific question.
      * @param string $summarystate the summary state of interest, or 'all'.
      * @param string $orderby the column to order by.
@@ -781,7 +789,7 @@ $sqlorderby
      *
      * @param qubaid_condition $qubaids used to restrict which usages are included
      * in the query. See {@link qubaid_condition}.
-     * @param array $slots if null, load info for all quesitions, otherwise only
+     * @param array|null $slots if null, load info for all quesitions, otherwise only
      * load the averages for the specified questions.
      * @return array of objects with fields ->slot, ->averagefraction and ->numaveraged.
      */
@@ -1021,11 +1029,11 @@ ORDER BY
     }
 
     /**
-     * Delete all the steps for a question attempt.
+     * Delete some steps of a question attempt.
      *
      * Private method, only for use by other parts of the question engine.
      *
-     * @param int $qaids question_attempt id.
+     * @param array $stepids array of step ids to delete.
      * @param context $context the context that the $quba belongs to.
      */
     public function delete_steps($stepids, $context) {
@@ -1086,7 +1094,8 @@ ORDER BY
      *
      * @param int $qubaid the question usage id.
      * @param int $questionid the question id.
-     * @param int $sessionid the question_attempt id.
+     * @param int $qaid the question_attempt id.
+     * @param int $slot the slot number of the question attempt to update.
      * @param bool $newstate the new state of the flag. true = flagged.
      */
     public function update_question_attempt_flag($qubaid, $questionid, $qaid, $slot, $newstate) {
@@ -1102,7 +1111,8 @@ ORDER BY
      * Get all the WHEN 'x' THEN 'y' terms needed to convert the question_attempt_steps.state
      * column to a summary state. Use this like
      * CASE qas.state {$this->full_states_to_summary_state_sql()} END AS summarystate,
-     * @param string SQL fragment.
+     *
+     * @return string SQL fragment.
      */
     protected function full_states_to_summary_state_sql() {
         $sql = '';
@@ -1121,7 +1131,8 @@ ORDER BY
      * @param string $summarystate one of
      * inprogress, needsgrading, manuallygraded or autograded
      * @param bool $equal if false, do a NOT IN test. Default true.
-     * @return string SQL fragment.
+     * @param string $prefix used in the call to $DB->get_in_or_equal().
+     * @return array as returned by $DB->get_in_or_equal().
      */
     public function in_summary_state_test($summarystate, $equal = true, $prefix = 'summarystates') {
         $states = question_state::get_all_for_summary_state($summarystate);
@@ -1309,17 +1320,23 @@ class question_engine_unit_of_work implements question_usage_observer {
     protected $modified = false;
 
     /**
-     * @var array list of slot => {@link question_attempt}s that
+     * @var question_attempt[] list of slot => {@link question_attempt}s that
      * have been added to the usage.
      */
     protected $attemptsadded = array();
 
     /**
-     * @var array list of slot => {@link question_attempt}s that
+     * @var question_attempt[] list of slot => {@link question_attempt}s that
      * were already in the usage, and which have been modified.
      */
     protected $attemptsmodified = array();
 
+    /**
+     * @var question_attempt[] list of slot => {@link question_attempt}s that
+     * have been added to the usage.
+     */
+    protected $attemptsdeleted = array();
+
     /**
      * @var array of array(question_attempt_step, question_attempt id, seq number)
      * of steps that have been added to question attempts in this usage.
@@ -1333,7 +1350,7 @@ class question_engine_unit_of_work implements question_usage_observer {
     protected $stepsmodified = array();
 
     /**
-     * @var array list of question_attempt_step.id => question_attempt_step of steps
+     * @var question_attempt_step[] list of question_attempt_step.id => question_attempt_step of steps
      * that were previously stored in the database, but which are no longer required.
      */
     protected $stepsdeleted = array();
@@ -1506,13 +1523,15 @@ class question_engine_unit_of_work implements question_usage_observer {
     }
 
     /**
+     * Determine if a step is new. If so get its array key.
+     *
      * @param question_attempt_step $step a step
      * @return int|false if the step is in the list of steps to be added, return
      *      the key, otherwise return false.
      */
     protected function is_step_added(question_attempt_step $step) {
         foreach ($this->stepsadded as $key => $data) {
-            list($addedstep, $qaid, $seq) = $data;
+            list($addedstep) = $data;
             if ($addedstep === $step) {
                 return $key;
             }
@@ -1521,13 +1540,15 @@ class question_engine_unit_of_work implements question_usage_observer {
     }
 
     /**
+     * Determine if a step is modified. If so get its array key.
+     *
      * @param question_attempt_step $step a step
      * @return int|false if the step is in the list of steps to be modified, return
      *      the key, otherwise return false.
      */
     protected function is_step_modified(question_attempt_step $step) {
         foreach ($this->stepsmodified as $key => $data) {
-            list($modifiedstep, $qaid, $seq) = $data;
+            list($modifiedstep) = $data;
             if ($modifiedstep === $step) {
                 return $key;
             }
@@ -1648,10 +1669,12 @@ class question_file_saver implements question_response_files {
     protected $value = null;
 
     /**
-     * Constuctor.
+     * Constructor.
+     *
      * @param int $draftitemid the draft area to save the files from.
      * @param string $component the component for the file area to save into.
      * @param string $filearea the name of the file area to save into.
+     * @param string $text optional content containing file links.
      */
     public function __construct($draftitemid, $component, $filearea, $text = null) {
         $this->draftitemid = $draftitemid;
@@ -1661,10 +1684,13 @@ class question_file_saver implements question_response_files {
     }
 
     /**
-     * Compute the value that should be stored in the question_attempt_step_data
-     * table. Contains a hash that (almost) uniquely encodes all the files.
+     * Compute the value that should be stored in the question_attempt_step_data table.
+     *
+     * Contains a hash that (almost) uniquely encodes all the files.
+     *
      * @param int $draftitemid the draft file area itemid.
      * @param string $text optional content containing file links.
+     * @return string the value.
      */
     protected function compute_value($draftitemid, $text) {
         global $USER;
@@ -1708,7 +1734,9 @@ class question_file_saver implements question_response_files {
 
     /**
      * Actually save the files.
+     *
      * @param integer $itemid the item id for the file area to save into.
+     * @param context $context the context where the files should be saved.
      */
     public function save_files($itemid, $context) {
         file_save_draft_area_files($this->draftitemid, $context->id,
@@ -1836,9 +1864,14 @@ class question_file_loader implements question_response_files {
 abstract class qubaid_condition {
 
     /**
-     * @return string the SQL that needs to go in the FROM clause when trying
-     * to select records from the 'question_attempts' table based on the
+     * Get the SQL fragment to go in a FROM clause.
+     *
+     * The SQL that needs to go in the FROM clause when trying
+     * to select records from the 'question_attempts' table based on this
      * qubaid_condition.
+     *
+     * @param string $alias
+     * @return string SQL fragment.
      */
     public abstract function from_question_attempts($alias);
 
@@ -1846,7 +1879,7 @@ abstract class qubaid_condition {
     public abstract function where();
 
     /**
-     * @return the params needed by a query that uses
+     * @return array the params needed by a query that uses
      * {@link from_question_attempts()} and {@link where()}.
      */
     public abstract function from_where_params();
@@ -1858,7 +1891,7 @@ abstract class qubaid_condition {
     public abstract function usage_id_in();
 
     /**
-     * @return the params needed by a query that uses {@link usage_id_in()}.
+     * @return array the params needed by a query that uses {@link usage_id_in()}.
      */
     public abstract function usage_id_in_params();
 
@@ -1899,8 +1932,6 @@ class qubaid_list extends qubaid_condition {
     }
 
     public function where() {
-        global $DB;
-
         if (is_null($this->columntotest)) {
             throw new coding_exception('Must call from_question_attempts before where().');
         }
index 453fcd2..39182a1 100644 (file)
@@ -385,6 +385,7 @@ class question_usage_by_activity {
      * The values are arrays with two items, title and content. Each of these
      * will be either a string, or a renderable.
      *
+     * @param question_display_options $options display options to apply.
      * @return array as described above.
      */
     public function get_summary_information(question_display_options $options) {
@@ -393,21 +394,30 @@ class question_usage_by_activity {
     }
 
     /**
-     * @return string a simple textual summary of the question that was asked.
+     * Get a simple textual summary of the question that was asked.
+     *
+     * @param int $slot the slot number of the question to summarise.
+     * @return string the question summary.
      */
     public function get_question_summary($slot) {
         return $this->get_question_attempt($slot)->get_question_summary();
     }
 
     /**
-     * @return string a simple textual summary of response given.
+     * Get a simple textual summary of response given.
+     *
+     * @param int $slot the slot number of the question to get the response summary for.
+     * @return string the response summary.
      */
     public function get_response_summary($slot) {
         return $this->get_question_attempt($slot)->get_response_summary();
     }
 
     /**
-     * @return string a simple textual summary of the correct resonse.
+     * Get a simple textual summary of the correct response to a question.
+     *
+     * @param int $slot the slot number of the question to get the right answer summary for.
+     * @return string the right answer summary.
      */
     public function get_right_answer_summary($slot) {
         return $this->get_question_attempt($slot)->get_right_answer_summary();
@@ -499,9 +509,8 @@ class question_usage_by_activity {
      * For internal use only. Used when reloading the state of a question from the
      * database.
      *
-     * @param array $records Raw records loaded from the database.
-     * @param int $questionattemptid The id of the question_attempt to extract.
-     * @return question_attempt The newly constructed question_attempt_step.
+     * @param int $slot the slot number of the question to replace.
+     * @param question_attempt $qa the question attempt to put in that place.
      */
     public function replace_loaded_question_attempt_info($slot, $qa) {
         $this->check_slot($slot);
@@ -543,7 +552,7 @@ class question_usage_by_activity {
      * @param int $variant which variant of the question to use. Must be between
      *      1 and ->get_num_variants($slot) inclusive. If not give, a variant is
      *      chosen at random.
-     * @param int $timestamp optional, the timstamp to record for this action. Defaults to now.
+     * @param int|null $timenow optional, the timstamp to record for this action. Defaults to now.
      */
     public function start_question($slot, $variant = null, $timenow = null) {
         if (is_null($variant)) {
@@ -667,7 +676,7 @@ class question_usage_by_activity {
      * particular question.
      *
      * @param int $slot the number used to identify this question within this usage.
-     * @param $postdata optional, only intended for testing. Use this data
+     * @param array|null $postdata optional, only intended for testing. Use this data
      * instead of the data from $_POST.
      * @return array submitted data specific to this question.
      */
@@ -722,7 +731,8 @@ class question_usage_by_activity {
     /**
      * Process a specific action on a specific question.
      * @param int $slot the number used to identify this question within this usage.
-     * @param $submitteddata the submitted data that constitutes the action.
+     * @param array $submitteddata the submitted data that constitutes the action.
+     * @param int|null $timestamp (optional) the timestamp to consider 'now'.
      */
     public function process_action($slot, $submitteddata, $timestamp = null) {
         $qa = $this->get_question_attempt($slot);
@@ -733,7 +743,8 @@ class question_usage_by_activity {
     /**
      * Process an autosave action on a specific question.
      * @param int $slot the number used to identify this question within this usage.
-     * @param $submitteddata the submitted data that constitutes the action.
+     * @param array $submitteddata the submitted data that constitutes the action.
+     * @param int|null $timestamp (optional) the timestamp to consider 'now'.
      */
     public function process_autosave($slot, $submitteddata, $timestamp = null) {
         $qa = $this->get_question_attempt($slot);
@@ -743,12 +754,14 @@ class question_usage_by_activity {
     }
 
     /**
-     * Check that the sequence number, that detects weird things like the student
-     * clicking back, is OK. If the sequence check variable is not present, returns
+     * Check that the sequence number, that detects weird things like the student clicking back, is OK.
+     *
+     * If the sequence check variable is not present, returns
      * false. If the check variable is present and correct, returns true. If the
      * variable is present and wrong, throws an exception.
+     *
      * @param int $slot the number used to identify this question within this usage.
-     * @param array $submitteddata the submitted data that constitutes the action.
+     * @param array|null $postdata (optional) data to use in place of $_POST.
      * @return bool true if the check variable is present and correct. False if it
      * is missing. (Throws an exception if the check fails.)
      */
@@ -767,8 +780,9 @@ class question_usage_by_activity {
 
     /**
      * Check, based on the sequence number, whether this auto-save is still required.
+     *
      * @param int $slot the number used to identify this question within this usage.
-     * @param array $submitteddata the submitted data that constitutes the action.
+     * @param array|null $postdata the submitted data that constitutes the action.
      * @return bool true if the check variable is present and correct, otherwise false.
      */
     public function is_autosave_required($slot, $postdata = null) {
@@ -788,7 +802,7 @@ class question_usage_by_activity {
      * Update the flagged state for all question_attempts in this usage, if their
      * flagged state was changed in the request.
      *
-     * @param $postdata optional, only intended for testing. Use this data
+     * @param array|null $postdata optional, only intended for testing. Use this data
      * instead of the data from $_POST.
      */
     public function update_question_flags($postdata = null) {
@@ -824,6 +838,7 @@ class question_usage_by_activity {
      * manual grading, or changing the flag state.
      *
      * @param int $slot the number used to identify this question within this usage.
+     * @param int|null $timestamp (optional) the timestamp to consider 'now'.
      */
     public function finish_question($slot, $timestamp = null) {
         $qa = $this->get_question_attempt($slot);
@@ -834,6 +849,8 @@ class question_usage_by_activity {
     /**
      * Finish the active phase of an attempt at a question. See {@link finish_question()}
      * for a fuller description of what 'finish' means.
+     *
+     * @param int|null $timestamp (optional) the timestamp to consider 'now'.
      */
     public function finish_all_questions($timestamp = null) {
         foreach ($this->questionattempts as $qa) {
@@ -906,7 +923,7 @@ class question_usage_by_activity {
      * For internal use only.
      *
      * @param Iterator $records Raw records loaded from the database.
-     * @param int $questionattemptid The id of the question_attempt to extract.
+     * @param int $qubaid The id of the question usage we are loading.
      * @return question_usage_by_activity The newly constructed usage.
      */
     public static function load_from_records($records, $qubaid) {
@@ -952,8 +969,7 @@ class question_usage_by_activity {
 
 
 /**
- * A class abstracting access to the
- * {@link question_usage_by_activity::$questionattempts} array.
+ * A class abstracting access to the {@link question_usage_by_activity::$questionattempts} array.
  *
  * This class snapshots the list of {@link question_attempts} to iterate over
  * when it is created. If a question is added to the usage mid-iteration, it
@@ -966,15 +982,18 @@ class question_usage_by_activity {
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class question_attempt_iterator implements Iterator, ArrayAccess {
+
     /** @var question_usage_by_activity that we are iterating over. */
     protected $quba;
-    /** @var array of question numbers. */
+
+    /** @var array of slot numbers. */
     protected $slots;
 
     /**
      * To create an instance of this class, use
      * {@link question_usage_by_activity::get_attempt_iterator()}.
-     * @param $quba the usage to iterate over.
+     *
+     * @param question_usage_by_activity $quba the usage to iterate over.
      */
     public function __construct(question_usage_by_activity $quba) {
         $this->quba = $quba;
@@ -982,37 +1001,83 @@ class question_attempt_iterator implements Iterator, ArrayAccess {
         $this->rewind();
     }
 
-    /** @return question_attempt_step */
+    /**
+     * Standard part of the Iterator interface.
+     *
+     * @return question_attempt
+     */
     public function current() {
         return $this->offsetGet(current($this->slots));
     }
-    /** @return int */
+
+    /**
+     * Standard part of the Iterator interface.
+     *
+     * @return int
+     */
     public function key() {
         return current($this->slots);
     }
+
+    /**
+     * Standard part of the Iterator interface.
+     */
     public function next() {
         next($this->slots);
     }
+
+    /**
+     * Standard part of the Iterator interface.
+     */
     public function rewind() {
         reset($this->slots);
     }
-    /** @return bool */
+
+    /**
+     * Standard part of the Iterator interface.
+     *
+     * @return bool
+     */
     public function valid() {
         return current($this->slots) !== false;
     }
 
-    /** @return bool */
+    /**
+     * Standard part of the ArrayAccess interface.
+     *
+     * @param int $slot
+     * @return bool
+     */
     public function offsetExists($slot) {
         return in_array($slot, $this->slots);
     }
-    /** @return question_attempt_step */
+
+    /**
+     * Standard part of the ArrayAccess interface.
+     *
+     * @param int $slot
+     * @return question_attempt
+     */
     public function offsetGet($slot) {
         return $this->quba->get_question_attempt($slot);
     }
+
+    /**
+     * Standard part of the ArrayAccess interface.
+     *
+     * @param int $slot
+     * @param question_attempt $value
+     */
     public function offsetSet($slot, $value) {
         throw new coding_exception('You are only allowed read-only access to ' .
                 'question_attempt::states through a question_attempt_step_iterator. Cannot set.');
     }
+
+    /**
+     * Standard part of the ArrayAccess interface.
+     *
+     * @param int $slot
+     */
     public function offsetUnset($slot) {
         throw new coding_exception('You are only allowed read-only access to ' .
                 'question_attempt::states through a question_attempt_step_iterator. Cannot unset.');
index 79c40b2..e48f0d5 100644 (file)
@@ -72,7 +72,8 @@ abstract class question_state {
 
     /**
      * Get all the states in an array.
-     * @return of question_state objects.
+     *
+     * @return question_state[] of question_state objects.
      */
     public static function get_all() {
         $states = array();
@@ -87,7 +88,7 @@ abstract class question_state {
      * Get all the states in an array.
      * @param string $summarystate one of the four summary states
      * inprogress, needsgrading, manuallygraded or autograded.
-     * @return arrau of the corresponding states.
+     * @return array of the corresponding states.
      */
     public static function get_all_for_summary_state($summarystate) {
         $states = array();
index 06ccce4..2707f91 100644 (file)
@@ -119,19 +119,24 @@ class question_engine_data_mapper_reporting_testcase extends qbehaviour_walkthro
         $this->bothusages = new qubaid_list($this->usageids);
 
         // Now test the various queries.
-        $this->dotest_load_questions_usages_latest_steps();
-        $this->dotest_load_questions_usages_question_state_summary();
+        $this->dotest_load_questions_usages_latest_steps($this->allslots);
+        $this->dotest_load_questions_usages_latest_steps(null);
+        $this->dotest_load_questions_usages_question_state_summary($this->allslots);
+        $this->dotest_load_questions_usages_question_state_summary(null);
         $this->dotest_load_questions_usages_where_question_in_state();
-        $this->dotest_load_average_marks();
+        $this->dotest_load_average_marks($this->allslots);
+        $this->dotest_load_average_marks(null);
         $this->dotest_sum_usage_marks_subquery();
         $this->dotest_question_attempt_latest_state_view();
     }
 
     /**
      * This test is executed by {@link test_reporting_queries()}.
+     *
+     * @param array|null $slots list of slots to use in the call.
      */
-    protected function dotest_load_questions_usages_latest_steps() {
-        $rawstates = $this->dm->load_questions_usages_latest_steps($this->bothusages, $this->allslots,
+    protected function dotest_load_questions_usages_latest_steps($slots) {
+        $rawstates = $this->dm->load_questions_usages_latest_steps($this->bothusages, $slots,
                 'qa.id AS questionattemptid, qa.questionusageid, qa.slot, ' .
                 'qa.questionid, qa.maxmark, qas.sequencenumber, qas.state');
 
@@ -178,10 +183,12 @@ class question_engine_data_mapper_reporting_testcase extends qbehaviour_walkthro
 
     /**
      * This test is executed by {@link test_reporting_queries()}.
+     *
+     * @param array|null $slots list of slots to use in the call.
      */
-    protected function dotest_load_questions_usages_question_state_summary() {
+    protected function dotest_load_questions_usages_question_state_summary($slots) {
         $summary = $this->dm->load_questions_usages_question_state_summary(
-                $this->bothusages, $this->allslots);
+                $this->bothusages, $slots);
 
         $this->assertEquals($summary[$this->allslots[0] . ',' . $this->sa->id],
                 (object) array(
@@ -229,9 +236,11 @@ class question_engine_data_mapper_reporting_testcase extends qbehaviour_walkthro
 
     /**
      * This test is executed by {@link test_reporting_queries()}.
+     *
+     * @param array|null $slots list of slots to use in the call.
      */
-    protected function dotest_load_average_marks() {
-        $averages = $this->dm->load_average_marks($this->bothusages);
+    protected function dotest_load_average_marks($slots) {
+        $averages = $this->dm->load_average_marks($this->bothusages, $slots);
 
         $this->assertEquals(array(
             $this->allslots[0] => (object) array(
index 5e82530..0177abf 100644 (file)
@@ -575,14 +575,22 @@ class qformat_default {
                 $parent = $category->id;
             } else if ($category = $DB->get_record('question_categories',
                     array('name' => $catname, 'contextid' => $context->id, 'parent' => $parent))) {
-                // Do nothing unless the child category appears before the parent category
-                // in the imported xml file. Because the parent was created without info being available
-                // at that time, this allows the info to be added from the xml data.
-                if ($key == (count($catnames) - 1) && $lastcategoryinfo && isset($lastcategoryinfo->info) &&
-                        $lastcategoryinfo->info !== '' && $category->info === '') {
-                    $category->info = $lastcategoryinfo->info;
-                    if (isset($lastcategoryinfo->infoformat) && $lastcategoryinfo->infoformat !== '') {
-                        $category->infoformat = $lastcategoryinfo->infoformat;
+                // If this category is now the last one in the path we are processing ...
+                if ($key == (count($catnames) - 1) && $lastcategoryinfo) {
+                    // Do nothing unless the child category appears before the parent category
+                    // in the imported xml file. Because the parent was created without info being available
+                    // at that time, this allows the info to be added from the xml data.
+                    if (isset($lastcategoryinfo->info) && $lastcategoryinfo->info !== ''
+                            && $category->info === '') {
+                        $category->info = $lastcategoryinfo->info;
+                        if (isset($lastcategoryinfo->infoformat) && $lastcategoryinfo->infoformat !== '') {
+                            $category->infoformat = $lastcategoryinfo->infoformat;
+                        }
+                    }
+                    // Same for idnumber.
+                    if (isset($lastcategoryinfo->idnumber) && $lastcategoryinfo->idnumber !== ''
+                            && $category->idnumber === '') {
+                        $category->idnumber = $lastcategoryinfo->idnumber;
                     }
                     $DB->update_record('question_categories', $category);
                 }
@@ -603,11 +611,16 @@ class qformat_default {
                 $category->name = $catname;
                 $category->info = '';
                 // Only add info (category description) for the final category in the catpath.
-                if ($key == (count($catnames) - 1) && $lastcategoryinfo && isset($lastcategoryinfo->info) &&
-                        $lastcategoryinfo->info !== '') {
-                    $category->info = $lastcategoryinfo->info;
-                    if (isset($lastcategoryinfo->infoformat) && $lastcategoryinfo->infoformat !== '') {
-                        $category->infoformat = $lastcategoryinfo->infoformat;
+                if ($key == (count($catnames) - 1) && $lastcategoryinfo) {
+                    if (isset($lastcategoryinfo->info) && $lastcategoryinfo->info !== '') {
+                        $category->info = $lastcategoryinfo->info;
+                        if (isset($lastcategoryinfo->infoformat) && $lastcategoryinfo->infoformat !== '') {
+                            $category->infoformat = $lastcategoryinfo->infoformat;
+                        }
+                    }
+                    // Same for idnumber.
+                    if (isset($lastcategoryinfo->idnumber) && $lastcategoryinfo->idnumber !== '') {
+                        $category->idnumber = $lastcategoryinfo->idnumber;
                     }
                 }
                 $category->parent = $parent;
@@ -924,7 +937,7 @@ class qformat_default {
                         if (!count($DB->get_records('question', array('category' => $trackcategoryparent)))) {
                             $categoryname = $this->get_category_path($trackcategoryparent, $this->contexttofile);
                             $categoryinfo = $DB->get_record('question_categories', array('id' => $trackcategoryparent),
-                                'name, info, infoformat', MUST_EXIST);
+                                'name, info, infoformat, idnumber', MUST_EXIST);
                             if ($categoryinfo->name != 'top') {
                                 // Create 'dummy' question for parent category.
                                 $dummyquestion = $this->create_dummy_question_representing_category($categoryname, $categoryinfo);
@@ -937,7 +950,7 @@ class qformat_default {
                 if ($addnewcat && !in_array($trackcategory, $writtencategories)) {
                     $categoryname = $this->get_category_path($trackcategory, $this->contexttofile);
                     $categoryinfo = $DB->get_record('question_categories', array('id' => $trackcategory),
-                            'info, infoformat', MUST_EXIST);
+                            'info, infoformat, idnumber', MUST_EXIST);
                     // Create 'dummy' question for category.
                     $dummyquestion = $this->create_dummy_question_representing_category($categoryname, $categoryinfo);
                     $expout .= $this->writequestion($dummyquestion) . "\n";
@@ -986,6 +999,7 @@ class qformat_default {
         $dummyquestion->contextid = 0;
         $dummyquestion->info = $categoryinfo->info;
         $dummyquestion->infoformat = $categoryinfo->infoformat;
+        $dummyquestion->idnumber = $categoryinfo->idnumber;
         $dummyquestion->name = 'Switch category to ' . $categoryname;
         return $dummyquestion;
     }
index 98fe6d2..d2323d0 100644 (file)
@@ -935,6 +935,7 @@ class qformat_xml extends qformat_default {
             // The import should have the format in human readable form, so translate to machine readable format.
             $qo->infoformat = $this->trans_format($question['#']['info'][0]['@']['format']);
         }
+        $qo->idnumber = $this->getpath($question, array('#', 'idnumber', 0, '#'), null);
         return $qo;
     }
 
@@ -1205,6 +1206,7 @@ class qformat_xml extends qformat_default {
             $expout .= "    <info {$infoformat}>\n";
             $expout .= "      {$categoryinfo}";
             $expout .= "    </info>\n";
+            $expout .= "    <idnumber>{$question->idnumber}</idnumber>\n";
             $expout .= "  </question>\n";
             return $expout;
         }
index 5c8ff83..674457b 100644 (file)
@@ -8,6 +8,7 @@
     <info format="moodle_auto_format">
         <text>This is Alpha category for test</text>
     </info>
+    <idnumber>alpha-idnumber</idnumber>
   </question>
 
 <!-- question: 91  -->
index 42c9f37..63087cf 100644 (file)
@@ -8,6 +8,7 @@
     <info format="moodle_auto_format">
       <text>This is Alpha category for test</text>
     </info>
+    <idnumber>alpha-idnumber</idnumber>
   </question>
 
 <!-- question: 91  -->
index 4bf5c2c..ac6db64 100644 (file)
@@ -8,6 +8,7 @@
     <info format="plain_text">
       <text>This is Delta category for test</text>
     </info>
+    <idnumber></idnumber>
   </question>
 
 <!-- question: 0  -->
@@ -18,6 +19,7 @@
     <info format="markdown">
       <text>This is Epsilon category for test</text>
     </info>
+    <idnumber></idnumber>
   </question>
 
 <!-- question: 0  -->
@@ -28,6 +30,7 @@
     <info format="moodle_auto_format">
       <text>This is Zeta category for test</text>
     </info>
+    <idnumber></idnumber>
   </question>
 
 <!-- question: 93  -->
index db729e9..8350c16 100644 (file)
@@ -8,6 +8,7 @@
     <info format="plain_text">
       <text>This is Iota category for test</text>
     </info>
+    <idnumber></idnumber>
   </question>
 
 <!-- question: 96  -->
@@ -47,6 +48,7 @@
     <info format="markdown">
       <text>This is Kappa category for test</text>
     </info>
+    <idnumber></idnumber>
   </question>
 
 <!-- question: 106  -->
     <info format="moodle_auto_format">
       <text>This is Lambda category for test</text>
     </info>
+    <idnumber></idnumber>
   </question>
 
 <!-- question: 98  -->
     <info format="moodle_auto_format">
       <text>This is Mu category for test</text>
     </info>
+    <idnumber></idnumber>
   </question>
 
 <!-- question: 99  -->
index a0af0af..4926bea 100644 (file)
@@ -38,8 +38,8 @@ class qformat_xml_import_export_test extends advanced_testcase {
     /**
      * Create object qformat_xml for test.
      * @param string $filename with name for testing file.
-     * @param object $course
-     * @return object qformat_xml.
+     * @param stdClass $course
+     * @return qformat_xml XML question format object.
      */
     public function create_qformat($filename, $course) {
         global $DB;
@@ -99,11 +99,12 @@ class qformat_xml_import_export_test extends advanced_testcase {
      * @param string $info imported category info field (description of category).
      * @param int $infoformat imported category info field format.
      */
-    public function assert_category_imported($name, $info, $infoformat) {
+    public function assert_category_imported($name, $info, $infoformat, $idnumber = null) {
         global $DB;
         $category = $DB->get_record('question_categories', ['name' => $name], '*', MUST_EXIST);
         $this->assertEquals($info, $category->info);
         $this->assertEquals($infoformat, $category->infoformat);
+        $this->assertSame($idnumber, $category->idnumber);
     }
 
     /**
@@ -146,7 +147,8 @@ class qformat_xml_import_export_test extends advanced_testcase {
         $qformat = $this->create_qformat('category_with_description.xml', $course);
         $imported = $qformat->importprocess();
         $this->assertTrue($imported);
-        $this->assert_category_imported('Alpha', 'This is Alpha category for test', FORMAT_MOODLE);
+        $this->assert_category_imported('Alpha',
+                'This is Alpha category for test', FORMAT_MOODLE, 'alpha-idnumber');
         $this->assert_category_has_parent('Alpha', 'top');
     }
 
@@ -246,6 +248,7 @@ class qformat_xml_import_export_test extends advanced_testcase {
                 'contextid' => '2',
                 'info' => 'This is Alpha category for test',
                 'infoformat' => '0',
+                'idnumber' => 'alpha-idnumber',
                 'stamp' => make_unique_id_code(),
                 'parent' => '0',
                 'sortorder' => '999']);
index c85fff0..c8cd52c 100644 (file)
@@ -1721,6 +1721,7 @@ END;
         $categoryinfo = new stdClass();
         $categoryinfo->info = 'info1';
         $categoryinfo->infoformat = 'infoformat1';
+        $categoryinfo->idnumber = null;
         $dummyquestion = $testobject->mock_create_dummy_question_representing_category($categoryname, $categoryinfo);
 
         $this->assertEquals('category', $dummyquestion->qtype);
index 5d86833..9abddbc 100644 (file)
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2019100400.01;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2019100800.02;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 
-$release  = '3.8dev+ (Build: 20191004)'; // Human-friendly version name
+$release  = '3.8dev+ (Build: 20191008)'; // Human-friendly version name
 
 $branch   = '38';                       // This version's branch.
 $maturity = MATURITY_ALPHA;             // This version's maturity level.