#77 override proposal

amd/src/singleradiosync.js

@@ -0,0 +1,53 @@
+// 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/>.
+
+/**
+ * Used in single type questions with radio button groups so that each cell radio button
+ * that is selected syncs its selected column with the hidden inputs that hold the cell values
+ * stored for an attempt.
+ * E.g. a matrix has 4 columns, so 4 radio buttons per row.The radio button group name is row0.
+ * Each hidden cell input bears the cell's name (e.g. row0col2) and is either true or not.
+ * Those hidden input values are the response values that will be saved in attempt steps.
+ * This allows single and multiple to have the same response format and allows easier switching and
+ * helps during the regrading workflow.
+ * @copyright  2025
+ * @author Daniel Poggenpohl <daniel.poggenpohl@fernuni hagen.de>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+/**
+ *
+ * @param {Event} click
+ */
+const propagateChangedValue = (click) => {
+    let clickedElement = click.target;
+    if (clickedElement.tagName !== 'INPUT' || clickedElement.type !== 'radio') {
+        return;
+    }
+    let hiddenCheckboxForRadio = clickedElement.parentElement.querySelector('input[type="checkbox"]');
+    let hiddenCheckboxesForRow = clickedElement.closest('tr').querySelectorAll('input[type="checkbox"]');
+    hiddenCheckboxesForRow.forEach((hiddenCheckboxForRow) => {
+        hiddenCheckboxForRow.checked = false;
+    });
+    hiddenCheckboxForRadio.checked = true;
+};
+
+export const init = (matrixTableId) => {
+    let matrixTable = document.getElementById(matrixTableId);
+    let matrixRows = matrixTable.querySelectorAll('tbody tr');
+    matrixRows.forEach((matrixRow) => {
+        matrixRow.addEventListener('click', propagateChangedValue);
+    });
+};

backup/moodle2/restore_qtype_matrix_plugin.class.php

@@ -16,10 +16,15 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-use qtype_matrix\local\question_cleaner;
-
 global $CFG;
-require_once($CFG->dirroot . '/question/engine/bank.php');
+
+require_once $CFG->dirroot . '/question/engine/bank.php';
+require_once $CFG->dirroot . '/question/type/matrix/question.php';
+require_once $CFG->dirroot . '/question/type/matrix/questiontype.php';
+
+use qtype_matrix\db\question_matrix_store;
+use qtype_matrix\db\stepdata_migration_utils;
+use core\exception\moodle_exception;
 
 /**
  * restore plugin class that provides the necessary information
@@ -38,8 +43,6 @@ public static function define_decode_contents(): array {
         $result[] = new restore_decode_content('qtype_matrix_cols', $fields, 'qtype_matrix_cols');
         $fields = ['shorttext', 'description', 'feedback'];
         $result[] = new restore_decode_content('qtype_matrix_rows', $fields, 'qtype_matrix_rows');
-        $fields = ['rowid', 'colid', 'weight'];
-        $result[] = new restore_decode_content('qtype_matrix_weights', $fields, 'qtype_matrix_weights');
 
         return $result;
     }
@@ -56,8 +59,6 @@ public function process_matrix($data): void {
         $data = (object) $data;
         $oldid = $data->id;
 
-        // Todo: check import of version moodle1 data.
-
         if ($this->is_question_created()) {
             $qtypeobj = question_bank::get_qtype($this->pluginname);
             $data->{$qtypeobj->questionid_column_name()} = $this->get_new_parentid('question');
@@ -81,70 +82,49 @@ protected function is_question_created(): bool {
     }
 
     /**
-     * Process the qtype/cols/col
+     * Process the qtype/rows/row element.
      *
      * @param $data
      * @return void
      * @throws dml_exception
      */
-    public function process_col($data): void {
-        global $DB;
-        $data = (object) $data;
-        $oldid = $data->id;
-
-        $oldmatrixid = $this->get_old_parentid('matrix');
-        $newmatrixid = $this->get_new_parentid('matrix');
-        if (!$newmatrixid) {
-            return;
-        }
-
-        if ($this->is_question_created()) {
-            $data->matrixid = $newmatrixid;
-            $newitemid = $DB->insert_record('qtype_matrix_cols', $data);
-        } else {
-            $originalrecords = $DB->get_records('qtype_matrix_cols', ['matrixid' => $newmatrixid]);
-            foreach ($originalrecords as $record) {
-                if ($data->shorttext == $record->shorttext) { // Todo: this looks dirty to me!
-                    $newitemid = $record->id;
-                }
-            }
-        }
-        if (!isset($newitemid)) {
-            $info = new stdClass();
-            $info->filequestionid = $oldmatrixid;
-            $info->dbquestionid = $newmatrixid;
-            $info->answer = $data->shorttext;
-            throw new restore_step_exception('error_question_answers_missing_in_db', $info);
-        } else {
-            $this->set_mapping('col', $oldid, $newitemid);
-        }
+    public function process_row($data): void {
+        $this->process_dim($data, true);
     }
 
     /**
-     * Process the qtype/rows/row element
+     * Process the qtype/cols/col element.
      *
      * @param $data
      * @return void
      * @throws dml_exception
      */
-    public function process_row($data): void {
+    public function process_col($data): void {
+        $this->process_dim($data, false);
+    }
+
+    private function process_dim($backupdata, bool $isrow): void {
         global $DB;
-        $data = (object) $data;
-        $oldid = $data->id;
+        $backupdata = (object) $backupdata;
+        $oldid = $backupdata->id;
 
         $oldmatrixid = $this->get_old_parentid('matrix');
         $newmatrixid = $this->get_new_parentid('matrix');
         if (!$newmatrixid) {
             return;
         }
 
+        $dim = $isrow ? 'row' : 'col';
+        $dimtable = $isrow ? 'qtype_matrix_rows' : 'qtype_matrix_cols';
+        $newitemid = 0;
         if ($this->is_question_created()) {
-            $data->matrixid = $newmatrixid;
-            $newitemid = $DB->insert_record('qtype_matrix_rows', $data);
+            $backupdata->matrixid = $newmatrixid;
+            $newitemid = $DB->insert_record($dimtable, $backupdata);
         } else {
-            $originalrecords = $DB->get_records('qtype_matrix_rows', ['matrixid' => $newmatrixid]);
+            // FIXME: It isn't ensured that 2 rows/cols don't have the same shorttext right now.
+            $originalrecords = $DB->get_records($dimtable, ['matrixid' => $newmatrixid]);
             foreach ($originalrecords as $record) {
-                if ($data->shorttext == $record->shorttext) { // Todo: this looks dirty to me!
+                if ($backupdata->shorttext == $record->shorttext) {
                     $newitemid = $record->id;
                 }
             }
@@ -153,10 +133,10 @@ public function process_row($data): void {
             $info = new stdClass();
             $info->filequestionid = $oldmatrixid;
             $info->dbquestionid = $newmatrixid;
-            $info->answer = $data->shorttext;
+            $info->answer = $backupdata->shorttext;
             throw new restore_step_exception('error_question_answers_missing_in_db', $info);
         } else {
-            $this->set_mapping('row', $oldid, $newitemid);
+            $this->set_mapping($dim, $oldid, $newitemid);
         }
     }
 
@@ -175,6 +155,8 @@ public function process_weight($data): void {
         $key = $data->colid . 'x' . $data->rowid;
         $data->colid = $this->get_mappingid('col', $data->colid);
         $data->rowid = $this->get_mappingid('row', $data->rowid);
+        // This prevents bad data to arrive in the database. Currently the only useful weight values are 1 or 0.
+        $data->weight = (int) (bool) $data->weight;
         $newitemid = $DB->insert_record('qtype_matrix_weights', $data);
         $this->set_mapping('weight' . $key, $oldid, $newitemid);
     }
@@ -201,25 +183,52 @@ public function recode_legacy_state_answer($state): string {
         return serialize($result);
     }
 
+    /**
+     * Return a matrix store for database access.
+     * Exists mainly because unit tests work better with it.
+     * @return question_matrix_store
+     */
+    protected function get_matrix_store():question_matrix_store {
+        static $store = null;
+        if (!$store) {
+            $store = new question_matrix_store();
+        }
+        return $store;
+    }
+
     public function recode_response($questionid, $sequencenumber, array $response): array {
         $recodedresponse = [];
-        foreach ($response as $responsekey => $responseval) {
-            if ($responsekey == '_order') {
-                $recodedresponse['_order'] = $this->recode_choice_order($responseval);
-            } else if (substr($responsekey, 0, 4) == 'cell') {
-                $responsekeynocell = substr($responsekey, 4);
-                $responsekeyids = explode('_', $responsekeynocell);
-                $newrowid = $this->get_mappingid('row', $responsekeyids[0]);
-                $newcolid = $this->get_mappingid('col', $responseval) ?? false;
-                if (count($responsekeyids) == 1) {
-                    $recodedresponse['cell' . $newrowid] = $newcolid;
-                } else if (count($responsekeyids) == 2) {
-                    $recodedresponse['cell' . $newrowid . '_' . $newcolid] = $newcolid;
-                } else {
-                    $recodedresponse[$responsekey] = $responseval;
+
+        $store = $this->get_matrix_store();
+        $restoredmatrix = $store->get_matrix_by_question_id($questionid);
+        $restoredcols = $store->get_matrix_cols_by_matrix_id($restoredmatrix->id);
+        $restoredcolids = array_keys($restoredcols);
+
+        $oldorder = $response['_order'];
+        unset($response['_order']);
+        if (!$oldorder) {
+            throw new restore_step_exception('error_qtype_matrix_attempt_step_data_not_migratable');
+        }
+        $recodedresponse['_order'] = $this->recode_choice_order($oldorder);
+        $recodedorder = explode(',', $recodedresponse['_order']);
+        foreach ($response as $key => $value) {
+            if (str_contains($key, 'cell')) {
+                $oldrowid = stepdata_migration_utils::extract_row_id($key);
+                $newrowid = $this->get_mappingid('row', $oldrowid, 0);
+                $newrowindex = array_search($newrowid, $recodedorder);
+                $oldcolid = stepdata_migration_utils::extract_col_id($key, $value);
+                $newcolid = $this->get_mappingid('col', $oldcolid, 0);
+                $newcolindex = array_search($newcolid, $restoredcolids);
+                // Either we can map a backup ID to new rows/cols of a new question or to an already existing question.
+                // If we couldn't, then the row/col IDs from the attempt point to those of another earlier question version
+                // This version may or may not be in the backup and thus may or may not have been restored yet.
+                if ($newrowindex === false || $newcolindex === false) {
+                    throw new restore_step_exception('error_qtype_matrix_attempt_step_data_not_migratable');
                 }
+                $newname = qtype_matrix_question::responsekey($newrowindex, $newcolindex);
+                $recodedresponse[$newname] = true;
             } else {
-                $recodedresponse[$responsekey] = $responseval;
+                $recodedresponse[$key] = $value;
             }
         }
         return $recodedresponse;
@@ -234,7 +243,7 @@ public function recode_response($questionid, $sequencenumber, array $response):
     protected function recode_choice_order(string $order): string {
         $neworder = [];
         foreach (explode(',', $order) as $id) {
-            if ($newid = $this->get_mappingid('row', $id)) {
+            if ($newid = $this->get_mappingid('row', (int) $id)) {
                 $neworder[] = $newid;
             }
         }
@@ -275,10 +284,10 @@ protected function define_question_plugin_structure(): array {
      */
     public static function convert_backup_to_questiondata(array $backupdata): stdClass {
         $questiondata = parent::convert_backup_to_questiondata($backupdata);
-        $questiondata = question_cleaner::clean_data($questiondata, true);
+        $questiondata = qtype_matrix::clean_data($questiondata, true);
         // Add the matrix-specific options.
         if (isset($backupdata['plugin_qtype_matrix_question']['matrix'][0])) {
-            $matrix = $backupdata['plugin_qtype_matrix_question']['matrix'][0];
+            $matrix = &$backupdata['plugin_qtype_matrix_question']['matrix'][0];
 
             // Process rows to correct format
             $rowids = [];
@@ -348,22 +357,6 @@ public static function convert_backup_to_questiondata(array $backupdata): stdCla
         return $questiondata;
     }
 
-    /**
-     * Remove excluded fields from the questiondata structure. We use this function to remove the
-     * id and questionid fields for the weights, because they cannot be removed via the default
-     * mechanism due to the two-dimensional array. Once this is done, we call the parent function
-     * to remove the necessary fields.
-     *
-     * @param stdClass $questiondata
-     * @param array $excludefields Paths to the fields to exclude.
-     * @return stdClass The $questiondata with excluded fields removed.
-     */
-    public static function remove_excluded_question_data(stdClass $questiondata, array $excludefields = []): stdClass {
-        unset($questiondata->hints);
-
-        return parent::remove_excluded_question_data($questiondata, $excludefields);
-    }
-
     #[\Override]
     public function define_excluded_identity_hash_fields(): array {
         return [