diff --git a/.upgradenotes/MDL-83541-2025020615341872.yml b/.upgradenotes/MDL-83541-2025020615341872.yml new file mode 100644 index 00000000000..d729fe3cd9c --- /dev/null +++ b/.upgradenotes/MDL-83541-2025020615341872.yml @@ -0,0 +1,38 @@ +issueNumber: MDL-83541 +notes: + core_question: + - message: > + Duplication or multiple restores of questions has been modified to avoid + errors where a question with the same stamp already exists in the target + category. + + To achieve this all data for the question is hashed, excluding any ID + fields. + + The question data from the backup is first reformatted to match the questiondata + structure returned by calling `get_question_options()` (see + https://docs.moodle.org/dev/Question_data_structures#Representation_1:_%24questiondata). + Common question elements will be handled automatically, but any elements that + the qtype adds to the backup will need to be handled by overriding + `restore_qtype_plugin::convert_backup_to_questiondata`. See `restore_qtype_match_plugin` + as an example. + + If a qtype plugin calls any `$this->add_question_*()` methods in its + `restore_qtype_*_plugin::define_question_plugin_structure()` method, the + ID fields used in these records will be excluded automatically. + + If a qtype plugin defines its own tables with ID fields, it must define + `restore_qtype_*_plugin::define_excluded_identity_hash_fields()` to return + an array of paths to these fields within the question data. This should be + all that is required for the majority of plugins. + See the PHPDoc of `restore_qtype_plugin::define_excluded_identity_hash_fields()` + for a full explanation of how these paths should be defined, and + `restore_qtype_truefalse_plugin` for an example. + + If the data structure for a qtype returned by calling + `get_question_options()` contains data other than ID fields that are not + contained in the backup structure or vice-versa, it will need to + override `restore_qtype_*_plugin::remove_excluded_question_data()` + to remove the inconsistent data. See `restore_qtype_multianswer_plugin` as + an example. + type: fixed diff --git a/backup/moodle2/restore_qtype_plugin.class.php b/backup/moodle2/restore_qtype_plugin.class.php index 23d0a3dc076..a7ae157d772 100644 --- a/backup/moodle2/restore_qtype_plugin.class.php +++ b/backup/moodle2/restore_qtype_plugin.class.php @@ -47,6 +47,11 @@ abstract class restore_qtype_plugin extends restore_plugin { */ private $questionanswercacheid = null; + /** + * @var array List of fields to exclude form hashing during restore. + */ + protected array $excludedhashfields = []; + /** * Add to $paths the restore_path_elements needed * to handle question_answers for a given question @@ -62,6 +67,10 @@ abstract class restore_qtype_plugin extends restore_plugin { $elename = 'question_answer'; $elepath = $this->get_pathfor('/answers/answer'); // we used get_recommended_name() so this works $paths[] = new restore_path_element($elename, $elepath); + $this->exclude_identity_hash_fields([ + '/options/answers/id', + '/options/answers/question', + ]); } /** @@ -78,6 +87,10 @@ abstract class restore_qtype_plugin extends restore_plugin { $elename = 'question_numerical_unit'; $elepath = $this->get_pathfor('/numerical_units/numerical_unit'); // we used get_recommended_name() so this works $paths[] = new restore_path_element($elename, $elepath); + $this->exclude_identity_hash_fields([ + '/options/units/id', + '/options/units/question', + ]); } /** @@ -94,6 +107,7 @@ abstract class restore_qtype_plugin extends restore_plugin { $elename = 'question_numerical_option'; $elepath = $this->get_pathfor('/numerical_options/numerical_option'); // we used get_recommended_name() so this works $paths[] = new restore_path_element($elename, $elepath); + $this->exclude_identity_hash_fields(['/options/question']); } /** @@ -114,6 +128,21 @@ abstract class restore_qtype_plugin extends restore_plugin { $elename = 'question_dataset_item'; $elepath = $this->get_pathfor('/dataset_definitions/dataset_definition/dataset_items/dataset_item'); $paths[] = new restore_path_element($elename, $elepath); + $this->exclude_identity_hash_fields([ + '/options/datasets/id', + '/options/datasets/question', + '/options/datasets/category', + '/options/datasets/type', + '/options/datasets/items/id', + // The following fields aren't included in the backup or DB structure, but are parsed from the options field. + '/options/datasets/status', + '/options/datasets/distribution', + '/options/datasets/minimum', + '/options/datasets/maximum', + '/options/datasets/decimals', + // This field is set dynamically from the count of items in the dataset, it is not backed up. + '/options/datasets/number_of_items', + ]); } /** @@ -395,4 +424,183 @@ abstract class restore_qtype_plugin extends restore_plugin { return $contents; } + + /** + * Add fields to the list of fields excluded from hashing. + * + * This allows common methods to add fields to the exclusion list. + * + * @param array $fields + * @return void + */ + private function exclude_identity_hash_fields(array $fields): void { + $this->excludedhashfields = array_merge($this->excludedhashfields, $fields); + } + + /** + * Return fields to be excluded from hashing during restores. + * + * @return array + */ + final public function get_excluded_identity_hash_fields(): array { + return array_unique(array_merge( + $this->excludedhashfields, + $this->define_excluded_identity_hash_fields(), + )); + } + + /** + * Return a list of paths to fields to be removed from questiondata before creating an identity hash. + * + * Fields that should be excluded from common elements such as answers or numerical units that are used by the plugin will + * be excluded automatically. This method just needs to define any specific to this plugin, such as foreign keys used in the + * plugin's tables. + * + * The returned array should be a list of slash-delimited paths to locate the fields to be removed from the questiondata object. + * For example, if you want to remove the field `$questiondata->options->questionid`, the path would be '/options/questionid'. + * If a field in the path is an array, the rest of the path will be applied to each object in the array. So if you have + * `$questiondata->options->answers[]`, the path '/options/answers/id' will remove the 'id' field from each element of the + * 'answers' array. + * + * @return array + */ + protected function define_excluded_identity_hash_fields(): array { + return []; + } + + /** + * Convert the backup structure of this question type into a structure matching its question data + * + * This should take the hierarchical array of tags from the question's backup structure, and return a structure that matches + * that returned when calling {@see get_question_options()} for this question type. + * See https://docs.moodle.org/dev/Question_data_structures#Representation_1:_%24questiondata for an explanation of this + * structure. + * + * This data will then be used to produce an identity hash for comparison with questions in the database. + * + * This base implementation deals with all common backup elements created by the add_question_*_options() methods in this class, + * plus elements added by ::define_question_plugin_structure() named for the qtype. The question type will need to extend + * this function if ::define_question_plugin_structure() adds any other elements to the backup. + * + * @param array $backupdata The hierarchical array of tags from the backup. + * @return \stdClass The questiondata object. + */ + public static function convert_backup_to_questiondata(array $backupdata): \stdClass { + // Create an object from the top-level fields. + $questiondata = (object) array_filter($backupdata, fn($tag) => !is_array($tag)); + $qtype = $questiondata->qtype; + $questiondata->options = new stdClass(); + if (isset($backupdata["plugin_qtype_{$qtype}_question"][$qtype])) { + $questiondata->options = (object) $backupdata["plugin_qtype_{$qtype}_question"][$qtype][0]; + } + if (isset($backupdata["plugin_qtype_{$qtype}_question"]['answers'])) { + $questiondata->options->answers = array_map( + fn($answer) => (object) $answer, + $backupdata["plugin_qtype_{$qtype}_question"]['answers']['answer'], + ); + } + if (isset($backupdata["plugin_qtype_{$qtype}_question"]['numerical_options'])) { + $questiondata->options = (object) array_merge( + (array) $questiondata->options, + $backupdata["plugin_qtype_{$qtype}_question"]['numerical_options']['numerical_option'][0], + ); + } + if (isset($backupdata["plugin_qtype_{$qtype}_question"]['numerical_units'])) { + $questiondata->options->units = array_map( + fn($unit) => (object) $unit, + $backupdata["plugin_qtype_{$qtype}_question"]['numerical_units']['numerical_unit'], + ); + } + if (isset($backupdata["plugin_qtype_{$qtype}_question"]['dataset_definitions'])) { + $questiondata->options->datasets = array_map( + fn($dataset) => (object) $dataset, + $backupdata["plugin_qtype_{$qtype}_question"]['dataset_definitions']['dataset_definition'], + ); + } + if (isset($questiondata->options->datasets)) { + foreach ($questiondata->options->datasets as $dataset) { + if (isset($dataset->dataset_items)) { + $dataset->items = array_map( + fn($item) => (object) $item, + $dataset->dataset_items['dataset_item'], + ); + unset($dataset->dataset_items); + } + } + } + if (isset($backupdata['question_hints'])) { + $questiondata->hints = array_map( + fn($hint) => (object) $hint, + $backupdata['question_hints']['question_hint'], + ); + } + + return $questiondata; + } + + /** + * Remove excluded fields from the questiondata structure. + * + * This removes fields that will not match or not be present in the question data structure produced by + * {@see self::convert_backup_to_questiondata()} and {@see get_question_options()} (such as IDs), so that the remaining data can + * be used to produce an identity hash for comparing the two. + * + * For plugins, it should be sufficient to override {@see self::define_excluded_identity_hash_fields()} with a list of paths + * specific to the plugin type. Overriding this method is only necessary if the plugin's + * {@see question_type::get_question_options()} method adds additional data to the question that is not included in the backup. + * + * @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 { + // All questions will need to exclude 'id' (used by question and other tables), 'questionid' (used by hints and options), + // 'createdby' and 'modifiedby' (since they won't map between sites). + $defaultexcludes = [ + '/id', + '/createdby', + '/modifiedby', + '/hints/id', + '/hints/questionid', + '/options/id', + '/options/questionid', + ]; + $excludefields = array_unique(array_merge($excludefields, $defaultexcludes)); + + foreach ($excludefields as $excludefield) { + $pathparts = explode('/', ltrim($excludefield, '/')); + $data = $questiondata; + self::unset_excluded_fields($data, $pathparts); + } + + return $questiondata; + } + + /** + * Iterate through the elements of path to an excluded field, and unset the final element. + * + * If any of the elements in the path is an array, this is called recursively on each element in the array to unset fields + * in each child of the array. + * + * @param stdClass|array $data The questiondata object, or a subsection of it. + * @param array $pathparts The remaining elements in the path to the excluded field. + * @return void + */ + private static function unset_excluded_fields(stdClass|array $data, array $pathparts): void { + $element = array_shift($pathparts); + if (!isset($data->{$element})) { + // This element is not present in the data structure, nothing to unset. + return; + } + if (is_object($data->{$element})) { + self::unset_excluded_fields($data->{$element}, $pathparts); + } else if (is_array($data->{$element})) { + foreach ($data->{$element} as $item) { + self::unset_excluded_fields($item, $pathparts); + } + } else if (empty($pathparts)) { + // This is the last element of the path and it's a scalar value, unset it. + unset($data->{$element}); + } + } } diff --git a/backup/tests/quiz_restore_decode_links_test.php b/backup/tests/quiz_restore_decode_links_test.php index 8540d8eb95b..0ddcb0f083a 100644 --- a/backup/tests/quiz_restore_decode_links_test.php +++ b/backup/tests/quiz_restore_decode_links_test.php @@ -64,6 +64,8 @@ final class quiz_restore_decode_links_test extends \advanced_testcase { $questiondata = \question_bank::load_question_data($question->id); + $DB->set_field('question', 'questiontext', $CFG->wwwroot . '/mod/quiz/view.php?id=' . $quiz->cmid, ['id' => $question->id]); + $firstanswer = array_shift($questiondata->options->answers); $DB->set_field('question_answers', 'answer', $CFG->wwwroot . '/course/view.php?id=' . $course->id, ['id' => $firstanswer->id]); @@ -87,6 +89,7 @@ final class quiz_restore_decode_links_test extends \advanced_testcase { $questionids = []; foreach ($quizquestions as $quizquestion) { if ($quizquestion->questionid) { + $this->assertEquals($CFG->wwwroot . '/mod/quiz/view.php?id=' . $quiz->cmid, $quizquestion->questiontext); $questionids[] = $quizquestion->questionid; } } diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php index 42f3d18511a..db667a9f868 100644 --- a/backup/util/dbops/restore_dbops.class.php +++ b/backup/util/dbops/restore_dbops.class.php @@ -567,7 +567,7 @@ abstract class restore_dbops { * @return array A separate list of all error and warnings detected */ public static function prechek_precheck_qbanks_by_level($restoreid, $courseid, $userid, $samesite, $contextlevel) { - global $DB; + global $DB, $CFG; // To return any errors and warnings found $errors = array(); @@ -669,21 +669,37 @@ abstract class restore_dbops { } else { self::set_backup_ids_record($restoreid, 'question_category', $category->id, $matchcat->id, $targetcontext->id); $questions = self::restore_get_questions($restoreid, $category->id); + $transformer = self::get_backup_xml_transformer($courseid); // Collect all the questions for this category into memory so we only talk to the DB once. - $questioncache = $DB->get_records_sql_menu('SELECT q.stamp, q.id - FROM {question} q - JOIN {question_versions} qv - ON qv.questionid = q.id - JOIN {question_bank_entries} qbe - ON qbe.id = qv.questionbankentryid - JOIN {question_categories} qc - ON qc.id = qbe.questioncategoryid - WHERE qc.id = ?', array($matchcat->id)); + $recordset = $DB->get_recordset_sql( + "SELECT q.* + FROM {question} q + JOIN {question_versions} qv ON qv.questionid = q.id + JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid + JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid + WHERE qc.id = ?", + [$matchcat->id], + ); + + // Compute a hash of question and answer fields to differentiate between identical stamp-version questions. + $questioncache = []; + foreach ($recordset as $question) { + $question->export_process = true; // Include all question options required for export. + get_question_options($question); + unset($question->export_process); + // Remove some additional properties from get_question_options() that isn't included in backups + // before we produce the identity hash. + unset($question->categoryobject); + unset($question->questioncategoryid); + $cachekey = restore_questions_parser_processor::generate_question_identity_hash($question, $transformer); + $questioncache[$cachekey] = $question->id; + } + $recordset->close(); foreach ($questions as $question) { - if (isset($questioncache[$question->stamp])) { - $matchqid = $questioncache[$question->stamp]; + if (isset($questioncache[$question->questionhash])) { + $matchqid = $questioncache[$question->questionhash]; } else { $matchqid = false; } @@ -1918,6 +1934,22 @@ abstract class restore_dbops { private static function password_should_be_discarded(#[\SensitiveParameter] string $password): bool { return (bool) preg_match('/^[0-9a-f]{32}$/', $password); } + + /** + * Load required classes and return a backup XML transformer for the specified course. + * + * These classes may not have been loaded if we're only doing a restore in the current process, + * so make sure we have them here. + * + * @param int $courseid + * @return backup_xml_transformer + */ + protected static function get_backup_xml_transformer(int $courseid): backup_xml_transformer { + global $CFG; + require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); + require_once($CFG->dirroot . '/backup/moodle2/backup_plan_builder.class.php'); + return new backup_xml_transformer($courseid); + } } /* diff --git a/backup/util/helper/restore_questions_parser_processor.class.php b/backup/util/helper/restore_questions_parser_processor.class.php index be3d0c28108..6e634c1bda4 100644 --- a/backup/util/helper/restore_questions_parser_processor.class.php +++ b/backup/util/helper/restore_questions_parser_processor.class.php @@ -45,6 +45,9 @@ class restore_questions_parser_processor extends grouped_parser_processor { /** @var string XML path in the questions.xml to question elements within question_category (before Moodle 4.0). */ protected const LEGACY_QUESTION_SUBPATH = '/questions/question'; + /** @var string String for concatenating data into a string for hashing.*/ + protected const HASHDATA_SEPARATOR = '|HASHDATA|'; + /** @var string identifies the current restore. */ protected string $restoreid; @@ -52,13 +55,44 @@ class restore_questions_parser_processor extends grouped_parser_processor { protected int $lastcatid; public function __construct($restoreid) { + global $CFG; $this->restoreid = $restoreid; $this->lastcatid = 0; parent::__construct(); // Set the paths we are interested on $this->add_path(self::CATEGORY_PATH); - $this->add_path(self::CATEGORY_PATH . self::QUESTION_SUBPATH); - $this->add_path(self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH); + $this->add_path(self::CATEGORY_PATH . self::QUESTION_SUBPATH, true); + $this->add_path(self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH, true); + + // Add all sub-elements, including those from plugins, as grouped paths with the question tag so that + // we can create a hash of all question data for comparison with questions in the database. + $this->add_path(self::CATEGORY_PATH . self::QUESTION_SUBPATH . '/question_hints'); + $this->add_path(self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH . '/question_hints'); + $this->add_path(self::CATEGORY_PATH . self::QUESTION_SUBPATH . '/question_hints/question_hint'); + $this->add_path(self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH . '/question_hints/question_hint'); + + $connectionpoint = new restore_path_element('question', self::CATEGORY_PATH . self::QUESTION_SUBPATH); + foreach (\core\plugin_manager::instance()->get_plugins_of_type('qtype') as $qtype) { + $restore = $this->get_qtype_restore($qtype->name); + if (!$restore) { + continue; + } + $structure = $restore->define_plugin_structure($connectionpoint); + foreach ($structure as $element) { + $subpath = str_replace(self::CATEGORY_PATH . self::QUESTION_SUBPATH . '/', '', $element->get_path()); + $pathparts = explode('/', $subpath); + $path = self::CATEGORY_PATH . self::QUESTION_SUBPATH; + $legacypath = self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH; + foreach ($pathparts as $part) { + $path .= '/' . $part; + $legacypath .= '/' . $part; + if (!in_array($path, $this->paths)) { + $this->add_path($path); + $this->add_path($legacypath); + } + } + } + } } protected function dispatch_chunk($data) { @@ -73,10 +107,19 @@ class restore_questions_parser_processor extends grouped_parser_processor { // Prepare question record } else if ($data['path'] == self::CATEGORY_PATH . self::QUESTION_SUBPATH || $data['path'] == self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH) { - $info = (object)$data['tags']; + // Remove sub-elements from the question info we're going to save. + $info = (object) array_filter($data['tags'], fn($tag) => !is_array($tag)); $itemname = 'question'; $itemid = $info->id; $parentitemid = $this->lastcatid; + $restore = $this->get_qtype_restore($data['tags']['qtype']); + if ($restore) { + $questiondata = $restore->convert_backup_to_questiondata($data['tags']); + } else { + $questiondata = restore_qtype_plugin::convert_backup_to_questiondata($data['tags']); + } + // Store a hash of question fields for comparison with existing questions. + $info->questionhash = $this->generate_question_identity_hash($questiondata); // Not question_category nor question, impossible. Throw exception. } else { @@ -106,4 +149,80 @@ class restore_questions_parser_processor extends grouped_parser_processor { } return $cdata; } + + /** + * Load and instantiate the restore class for the given question type. + * + * If there is no restore class, null is returned. + * + * @param string $qtype The question type name (no qtype_ prefix) + * @return ?restore_qtype_plugin + */ + protected static function get_qtype_restore(string $qtype): ?restore_qtype_plugin { + global $CFG; + $step = new restore_quiz_activity_structure_step('questions', 'question.xml'); + $filepath = "{$CFG->dirroot}/question/type/{$qtype}/backup/moodle2/restore_qtype_{$qtype}_plugin.class.php"; + if (!file_exists($filepath)) { + return null; + } + require_once($filepath); + $restoreclass = "restore_qtype_{$qtype}_plugin"; + if (!class_exists($restoreclass)) { + return null; + } + return new $restoreclass('qtype', $qtype, $step); + } + + /** + * Given a data structure containing the data for a question, reduce it to a flat array and return a sha1 hash of the data. + * + * @param stdClass $questiondata An array containing all the data for a question, including hints and qtype plugin data. + * @param ?backup_xml_transformer $transformer If provided, run the backup transformer process on all text fields. This ensures + * that values from the database are compared like-for-like with encoded values from the backup. + * @return string A sha1 hash of all question data, normalised and concatenated together. + */ + public static function generate_question_identity_hash( + stdClass $questiondata, + ?backup_xml_transformer $transformer = null, + ): string { + $questiondata = clone($questiondata); + $restore = self::get_qtype_restore($questiondata->qtype); + if ($restore) { + $restore->define_plugin_structure(new restore_path_element('question', self::CATEGORY_PATH . self::QUESTION_SUBPATH)); + // Combine default exclusions with those specified by the plugin. + $questiondata = $restore->remove_excluded_question_data($questiondata, $restore->get_excluded_identity_hash_fields()); + } else { + // The qtype has no restore class, use the default reduction method. + $questiondata = restore_qtype_plugin::remove_excluded_question_data($questiondata); + } + + // Convert questiondata to a flat array of values. + $hashdata = []; + // Convert the object to a multi-dimensional array for compatibility with array_walk_recursive. + $questiondata = json_decode(json_encode($questiondata), true); + array_walk_recursive($questiondata, function($value) use (&$hashdata) { + // Normalise data types. Depending on where the data comes from, it may be a mixture of nulls, strings, + // ints and floats. Convert everything to strings, then all numbers to floats to ensure we are doing + // like-for-like comparisons without losing accuracy. + $value = (string) $value; + if (is_numeric($value)) { + $value = (float) ($value); + } else if (str_contains($value, "\r\n")) { + // Normalise line breaks. + $value = str_replace("\r\n", "\n", $value); + } + $hashdata[] = $value; + }); + + sort($hashdata, SORT_STRING); + $hashstring = implode(self::HASHDATA_SEPARATOR, $hashdata); + if ($transformer) { + $hashstring = $transformer->process($hashstring); + // Need to re-sort the hashdata with the transformed strings. + $hashdata = explode(self::HASHDATA_SEPARATOR, $hashstring); + sort($hashdata, SORT_STRING); + $hashstring = implode(self::HASHDATA_SEPARATOR, $hashdata); + } + return sha1($hashstring); + } } diff --git a/mod/quiz/tests/backup/repeated_restore_test.php b/mod/quiz/tests/backup/repeated_restore_test.php index 204f3f1c958..af0c2635fb0 100644 --- a/mod/quiz/tests/backup/repeated_restore_test.php +++ b/mod/quiz/tests/backup/repeated_restore_test.php @@ -152,4 +152,616 @@ final class repeated_restore_test extends advanced_testcase { $this->assertEquals($questionscourse2firstimport[$slot->slot]->questionid, $slot->questionid); } } + + /** + * Restore a copy of a quiz to the same course, using questions that include line breaks in the text. + */ + public function test_restore_question_with_linebreaks(): void { + global $USER; + $this->resetAfterTest(); + $this->setAdminUser(); + + // Step 1: Create two courses and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $course2 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $generator->enrol_user($teacher->id, $course2->id, 'editingteacher'); + + // Create a quiz with questions in the first course. + $quiz = $this->create_test_quiz($course1); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create questions and add to the quiz. + $q1 = $questiongenerator->create_question('truefalse', null, [ + 'category' => $cat->id, + 'questiontext' => ['text' => "

Question

\r\n

One

", 'format' => FORMAT_MOODLE] + ]); + $q2 = $questiongenerator->create_question('truefalse', null, [ + 'category' => $cat->id, + 'questiontext' => ['text' => "

Question

\n

Two

", 'format' => FORMAT_MOODLE] + ]); + // Add question to quiz. + quiz_add_quiz_question($q1->id, $quiz); + quiz_add_quiz_question($q2->id, $quiz); + + // Capture original question IDs for verification after import. + $modules1 = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $module1 = reset($modules1); + $originalslots = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $module1->instance, $module1->context); + + $originalquestionids = []; + foreach ($originalslots as $slot) { + array_push($originalquestionids, intval($slot->questionid)); + } + + $this->assertCount(2, get_questions_category($cat, false)); + + // Step 2: Backup the quiz + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Step 3: Import the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify the question ids from the new quiz match the first. + $modules2 = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules2); + $module2 = end($modules2); + $copyslots = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $module2->instance, $module2->context); + + foreach ($copyslots as $slot) { + $this->assertContains(intval($slot->questionid), $originalquestionids); + } + + // The category should still only contain 2 question, neither question should be duplicated. + $this->assertCount(2, get_questions_category($cat, false)); + } + + /** + * Return a list of qtypes with valid generators in their helper class. + * + * This will check all installed qtypes for a test helper class, then find a defined test question which has a corresponding + * form_data method and return it. If the helper doesn't have a form_data method for any test question, it will return a + * null test question name for that qtype. + * + * @return array + */ + public static function get_qtype_generators(): array { + global $CFG; + $generators = []; + foreach (\core\plugin_manager::instance()->get_plugins_of_type('qtype') as $qtype) { + if ($qtype->name == 'random') { + continue; + } + $helperpath = "{$CFG->dirroot}/question/type/{$qtype->name}/tests/helper.php"; + if (!file_exists($helperpath)) { + continue; + } + require_once($helperpath); + $helperclass = "qtype_{$qtype->name}_test_helper"; + if (!class_exists($helperclass)) { + continue; + } + $helper = new $helperclass(); + $testquestion = null; + foreach ($helper->get_test_questions() as $question) { + if (method_exists($helper, "get_{$qtype->name}_question_form_data_{$question}")) { + $testquestion = $question; + break; + } + } + $generators[$qtype->name] = [ + 'qtype' => $qtype->name, + 'testquestion' => $testquestion, + ]; + } + return $generators; + } + + /** + * Restore a quiz with questions of same stamp into the same course, but different answers. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_quiz_with_same_stamp_questions(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create 2 quizzes with 2 questions multichoice. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz1, 0); + + // Update question2 to have the same stamp as question1. + $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]); + + // Change the answers of the question2 to be different to question1. + $question2data = \question_bank::load_question_data($question2->id); + if (!isset($question2data->options->answers) || empty($question2data->options->answers)) { + $this->markTestSkipped( + "Cannot test edited answers for qtype_{$qtype} as it does not use answers.", + ); + } + foreach ($question2data->options->answers as $answer) { + $DB->set_field('question_answers', 'answer', 'edited', ['id' => $answer->id]); + } + + // Backup quiz1. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the newly-restored quiz uses the same question as quiz2. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz1->id, + \context_module::instance($quiz1->cmid), + ); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz2structure[1]->questionid, $quiz2structure[1]->questionid); + $this->assertEquals($quiz2structure[2]->questionid, $quiz2structure[2]->questionid); + } + + /** + * Restore a quiz with duplicate questions (same stamp and questions) into the same course. + * + * This is a contrived case, but this test serves as a control for the other tests in this class, proving that the hashing + * process will match an identical question. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_quiz_with_duplicate_questions(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create a quiz with 2 identical but separate questions. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz1, 0); + + // Update question2 to have the same times and stamp as question1. + $DB->update_record('question', [ + 'id' => $question2->id, + 'stamp' => $question1->stamp, + 'timecreated' => $question1->timecreated, + 'timemodified' => $question1->timemodified, + ]); + + // Backup quiz. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Expect that the restored quiz will have the second question in both its slots + // by virtue of identical stamp, version, and hash of question answer texts. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz2structure[1]->questionid, $quiz2structure[2]->questionid); + } + + /** + * Restore a quiz with questions that have the same stamp but different text. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_quiz_with_edited_questions(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create a quiz with 2 identical but separate questions. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1); + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + // Edit question 2 to have the same stamp and times as question1, but different text. + $DB->update_record('question', [ + 'id' => $question2->id, + 'questiontext' => 'edited', + 'stamp' => $question1->stamp, + 'timecreated' => $question1->timecreated, + 'timemodified' => $question1->timemodified, + ]); + quiz_add_quiz_question($question2->id, $quiz1); + + // Backup quiz. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // The quiz should contain both questions, as they have different text. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz2structure[1]->questionid, $question1->id); + $this->assertEquals($quiz2structure[2]->questionid, $question2->id); + } + + /** + * Restore a course to another course having questions with the same stamp in a shared question bank context category. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_course_with_same_stamp_questions(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create three courses and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $course2 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $generator->enrol_user($teacher->id, $course2->id, 'editingteacher'); + + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $systemcontext = \context_system::instance(); + $cat = $questiongenerator->create_question_category(['contextid' => $systemcontext->id]); + + // Create quiz with question. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + + $quiz2 = $this->create_test_quiz($course1); + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz2, 0); + + // Update question2 to have the same stamp as question1. + $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]); + + // Change the answers of the question2 to be different to question1. + $question2data = \question_bank::load_question_data($question2->id); + if (!isset($question2data->options->answers) || empty($question2data->options->answers)) { + $this->markTestSkipped( + "Cannot test edited answers for qtype_{$qtype} as it does not use answers.", + ); + } + foreach ($question2data->options->answers as $answer) { + $answer->answer = 'New answer ' . $answer->id; + $DB->update_record('question_answers', $answer); + } + + // Backup course1. + $bc = new backup_controller(backup::TYPE_1COURSE, $course1->id, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup, adding to course2. + $rc = new restore_controller($backupid, $course2->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the newly-restored course's quizzes use the same questions as their counterparts of course1. + $modules = get_fast_modinfo($course2->id)->get_instances_of('quiz'); + $course1structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz1->id, \context_module::instance($quiz1->cmid)); + $course2quiz1 = array_shift($modules); + $course2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $course2quiz1->instance, $course2quiz1->context); + $this->assertEquals($course1structure[1]->questionid, $course2structure[1]->questionid); + + $course1structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz2->id, \context_module::instance($quiz2->cmid)); + $course2quiz2 = array_shift($modules); + $course2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $course2quiz2->instance, $course2quiz2->context); + $this->assertEquals($course1structure[1]->questionid, $course2structure[1]->questionid); + } + + /** + * Restore a quiz with questions of same stamp into the same course, but different hints. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_quiz_with_same_stamp_questions_edited_hints(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create 2 questions multichoice. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz1, 0); + + // Update question2 to have the same stamp as question1. + $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]); + + // Change the hints of the question2 to be different to question1. + $hints = $DB->get_records('question_hints', ['questionid' => $question2->id]); + if (empty($hints)) { + $this->markTestSkipped( + "Cannot test edited hints for qtype_{$qtype} as test question {$testquestion} does not use hints.", + ); + } + foreach ($hints as $hint) { + $DB->set_field('question_hints', 'hint', "{$hint->hint} edited", ['id' => $hint->id]); + } + + // Backup quiz1. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the newly-restored quiz uses the same question as quiz2. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz1structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz1->id, + \context_module::instance($quiz1->cmid), + ); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz1structure[1]->questionid, $quiz2structure[1]->questionid); + $this->assertEquals($quiz1structure[2]->questionid, $quiz2structure[2]->questionid); + + } + + /** + * Return a set of options fields and new values. + * + * @return array + */ + public static function get_edited_option_fields(): array { + return [ + 'single' => [ + 'single', + '0', + ], + 'shuffleanswers' => [ + 'shuffleanswers', + '0', + ], + 'answernumbering' => [ + 'answernumbering', + 'ABCD', + ], + 'shownumcorrect' => [ + 'shownumcorrect', + '0', + ], + 'showstandardinstruction' => [ + 'showstandardinstruction', + '1', + ], + 'correctfeedback' => [ + 'correctfeedback', + 'edited', + ], + 'partiallycorrectfeedback' => [ + 'partiallycorrectfeedback', + 'edited', + ], + 'incorrectfeedback' => [ + 'incorrectfeedback', + 'edited', + ], + ]; + } + + /** + * Restore a quiz with questions of same stamp into the same course, but different qtype-specific options. + * + * @dataProvider get_edited_option_fields + * @param string $field The answer field to edit + * @param string $value The value to set + */ + public function test_restore_quiz_with_same_stamp_questions_edited_options(string $field, string $value): void { + global $DB, $USER; + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // A quiz with 2 multichoice questions. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question('multichoice', 'one_of_four', ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + + $question2 = $questiongenerator->create_question('multichoice', 'one_of_four', ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz1, 0); + + // Update question2 to have the same stamp as question1. + $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]); + + // Change the options of question2 to be different to question1. + $DB->set_field('qtype_multichoice_options', $field, $value, ['questionid' => $question2->id]); + + // Backup quiz. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the newly-restored quiz questions match their quiz1 counterparts. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz1structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz1->id, + \context_module::instance($quiz1->cmid), + ); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz1structure[1]->questionid, $quiz2structure[1]->questionid); + $this->assertEquals($quiz1structure[2]->questionid, $quiz2structure[2]->questionid); + } } diff --git a/question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php b/question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php index 58d20e53c06..eb6b149d610 100644 --- a/question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php +++ b/question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php @@ -72,7 +72,7 @@ class backup_qtype_calculated_plugin extends backup_qtype_plugin { $calculatedoption = new backup_nested_element('calculated_option', array('id'), array( 'synchronize', 'single', 'shuffleanswers', 'correctfeedback', 'correctfeedbackformat', 'partiallycorrectfeedback', 'partiallycorrectfeedbackformat', - 'incorrectfeedback', 'incorrectfeedbackformat', 'answernumbering')); + 'incorrectfeedback', 'incorrectfeedbackformat', 'answernumbering', 'shownumcorrect')); // Now the own qtype tree. $pluginwrapper->add_child($calculatedrecords); diff --git a/question/type/calculated/backup/moodle2/restore_qtype_calculated_plugin.class.php b/question/type/calculated/backup/moodle2/restore_qtype_calculated_plugin.class.php index f9afd70ac68..5092dacc1a9 100644 --- a/question/type/calculated/backup/moodle2/restore_qtype_calculated_plugin.class.php +++ b/question/type/calculated/backup/moodle2/restore_qtype_calculated_plugin.class.php @@ -113,4 +113,46 @@ class restore_qtype_calculated_plugin extends restore_qtype_plugin { $newitemid = $DB->insert_record('question_calculated_options', $data); } } + + #[\Override] + public static function convert_backup_to_questiondata(array $backupdata): \stdClass { + $questiondata = parent::convert_backup_to_questiondata($backupdata); + $qtype = $questiondata->qtype; + foreach ($backupdata["plugin_qtype_{$qtype}_question"]['calculated_records']['calculated_record'] as $record) { + foreach ($questiondata->options->answers as &$answer) { + if ($answer->id == $record['answer']) { + $answer->tolerance = $record['tolerance']; + $answer->tolerancetype = $record['tolerancetype']; + $answer->correctanswerlength = $record['correctanswerlength']; + $answer->correctanswerformat = $record['correctanswerformat']; + continue 2; + } + } + } + if (isset($backupdata["plugin_qtype_{$qtype}_question"]['calculated_options'])) { + $questiondata->options = (object) array_merge( + (array) $questiondata->options, + $backupdata["plugin_qtype_{$qtype}_question"]['calculated_options']['calculated_option'][0], + ); + } + return $questiondata; + } + + #[\Override] + protected function define_excluded_identity_hash_fields(): array { + return [ + // These option fields are present in the database, but are only used by calculatedmulti. + '/options/synchronize', + '/options/single', + '/options/shuffleanswers', + '/options/correctfeedback', + '/options/correctfeedbackformat', + '/options/partiallycorrectfeedback', + '/options/partiallycorrectfeedbackformat', + '/options/incorrectfeedback', + '/options/incorrectfeedbackformat', + '/options/answernumbering', + '/options/shownumcorrect', + ]; + } } diff --git a/question/type/calculated/tests/helper.php b/question/type/calculated/tests/helper.php index cbbe0618021..8b93bfbf3cb 100644 --- a/question/type/calculated/tests/helper.php +++ b/question/type/calculated/tests/helper.php @@ -133,7 +133,7 @@ class qtype_calculated_test_helper extends question_test_helper { $fromform->defaultmark = 1.0; $fromform->generalfeedback = 'Generalfeedback: {={a} + {b}} is the right answer.'; - $fromform->unitrole = '3'; + $fromform->unitrole = '0'; $fromform->unitpenalty = 0.1; $fromform->unitgradingtypes = '1'; $fromform->unitsleft = '0'; @@ -187,6 +187,20 @@ class qtype_calculated_test_helper extends question_test_helper { $fromform->status = \core_question\local\bank\question_version_status::QUESTION_STATUS_READY; + $fromform->hint = [ + [ + 'text' => 'Add', + 'format' => FORMAT_HTML, + ], + ]; + + $fromform->unit = [ + 'x', + ]; + $fromform->multiplier = [ + '1.0', + ]; + return $fromform; } diff --git a/question/type/calculated/tests/question_type_test.php b/question/type/calculated/tests/question_type_test.php index b3cf88561c3..6f74e5686fd 100644 --- a/question/type/calculated/tests/question_type_test.php +++ b/question/type/calculated/tests/question_type_test.php @@ -106,12 +106,18 @@ final class question_type_test extends \advanced_testcase { $this->assertEquals($question->createdby, $questiondata->modifiedby); $this->assertEquals('', $questiondata->idnumber); $this->assertEquals($syscontext->id, $questiondata->contextid); - $this->assertEquals([], $questiondata->hints); + $this->assertCount(1, $questiondata->hints); + $hint = array_pop($questiondata->hints); + $this->assertEquals('Add', $hint->hint); + $this->assertEquals(FORMAT_HTML, $hint->hintformat); // Options. $this->assertEquals($questiondata->id, $questiondata->options->question); - $this->assertEquals([], $questiondata->options->units); - $this->assertEquals(qtype_numerical::UNITNONE, $questiondata->options->showunits); + $this->assertCount(1, $questiondata->options->units); + $unit = array_pop($questiondata->options->units); + $this->assertEquals($unit->unit, 'x'); + $this->assertEquals($unit->multiplier, '1.0'); + $this->assertEquals(qtype_numerical::UNITOPTIONAL, $questiondata->options->showunits); $this->assertEquals(0, $questiondata->options->unitgradingtype); // Unit role is none, so this is 0. $this->assertEquals($fromform->unitpenalty, $questiondata->options->unitpenalty); $this->assertEquals($fromform->unitsleft, $questiondata->options->unitsleft); diff --git a/question/type/calculatedmulti/backup/moodle2/restore_qtype_calculatedmulti_plugin.class.php b/question/type/calculatedmulti/backup/moodle2/restore_qtype_calculatedmulti_plugin.class.php index cc5b9ba1a70..a2b04cb9fa6 100644 --- a/question/type/calculatedmulti/backup/moodle2/restore_qtype_calculatedmulti_plugin.class.php +++ b/question/type/calculatedmulti/backup/moodle2/restore_qtype_calculatedmulti_plugin.class.php @@ -69,4 +69,9 @@ class restore_qtype_calculatedmulti_plugin extends restore_qtype_calculated_plug } return $result ? $result : $answer; } + + #[\Override] + protected function define_excluded_identity_hash_fields(): array { + return []; + } } diff --git a/question/type/calculatedmulti/tests/helper.php b/question/type/calculatedmulti/tests/helper.php index 92330cf2a92..6eafbd4c28e 100644 --- a/question/type/calculatedmulti/tests/helper.php +++ b/question/type/calculatedmulti/tests/helper.php @@ -127,4 +127,95 @@ class qtype_calculatedmulti_test_helper extends question_test_helper { return $q; } + + /** + * Return the form data for a question with a single response. + * + * @return stdClass + */ + public function get_calculatedmulti_question_form_data_singleresponse(): stdClass { + question_bank::load_question_definition_classes('calculated'); + $fromform = new stdClass(); + + $fromform->name = 'Simple sum'; + $fromform->questiontext['text'] = 'What is {a} + {b}?'; + $fromform->questiontext['format'] = FORMAT_HTML; + $fromform->defaultmark = 1.0; + $fromform->generalfeedback['text'] = 'Generalfeedback: {={a} + {b}} is the right answer.'; + $fromform->generalfeedback['format'] = FORMAT_HTML; + + $fromform->unitrole = '3'; + $fromform->unitpenalty = 0.1; + $fromform->unitgradingtypes = '1'; + $fromform->unitsleft = '0'; + $fromform->nounits = 1; + $fromform->multiplier = []; + $fromform->multiplier[0] = '1.0'; + $fromform->synchronize = 0; + $fromform->answernumbering = 0; + $fromform->shuffleanswers = 0; + $fromform->single = 1; + $fromform->correctfeedback['text'] = 'Very good'; + $fromform->correctfeedback['format'] = FORMAT_HTML; + $fromform->partiallycorrectfeedback['text'] = 'Mostly good'; + $fromform->partiallycorrectfeedback['format'] = FORMAT_HTML; + $fromform->incorrectfeedback['text'] = 'Completely Wrong'; + $fromform->incorrectfeedback['format'] = FORMAT_HTML; + $fromform->shownumcorrect = 1; + + $fromform->noanswers = 6; + $fromform->answer = []; + $fromform->answer[0]['text'] = '{a} + {b}'; + $fromform->answer[0]['format'] = FORMAT_HTML; + $fromform->answer[1]['text'] = '{a} - {b}'; + $fromform->answer[1]['format'] = FORMAT_HTML; + $fromform->answer[2]['text'] = '*'; + $fromform->answer[2]['format'] = FORMAT_HTML; + + $fromform->fraction = []; + $fromform->fraction[0] = '1.0'; + $fromform->fraction[1] = '0.0'; + $fromform->fraction[2] = '0.0'; + + $fromform->tolerance = []; + $fromform->tolerance[0] = 0.001; + $fromform->tolerance[1] = 0.001; + $fromform->tolerance[2] = 0; + + $fromform->tolerancetype[0] = 1; + $fromform->tolerancetype[1] = 1; + $fromform->tolerancetype[2] = 1; + + $fromform->correctanswerlength[0] = 2; + $fromform->correctanswerlength[1] = 2; + $fromform->correctanswerlength[2] = 2; + + $fromform->correctanswerformat[0] = 1; + $fromform->correctanswerformat[1] = 1; + $fromform->correctanswerformat[2] = 1; + + $fromform->feedback = []; + $fromform->feedback[0] = []; + $fromform->feedback[0]['format'] = FORMAT_HTML; + $fromform->feedback[0]['text'] = 'Very good.'; + + $fromform->feedback[1] = []; + $fromform->feedback[1]['format'] = FORMAT_HTML; + $fromform->feedback[1]['text'] = 'Add. not subtract!'; + + $fromform->feedback[2] = []; + $fromform->feedback[2]['format'] = FORMAT_HTML; + $fromform->feedback[2]['text'] = 'Completely wrong.'; + + $fromform->status = \core_question\local\bank\question_version_status::QUESTION_STATUS_READY; + + $fromform->hint = [ + [ + 'text' => 'Add', + 'format' => FORMAT_HTML, + ], + ]; + + return $fromform; + } } diff --git a/question/type/calculatedsimple/tests/helper.php b/question/type/calculatedsimple/tests/helper.php index eaa3d6af1fe..0cb15e3a1a3 100644 --- a/question/type/calculatedsimple/tests/helper.php +++ b/question/type/calculatedsimple/tests/helper.php @@ -205,6 +205,13 @@ class qtype_calculatedsimple_test_helper extends question_test_helper { $form->definition[19] = '1-0-b'; $form->definition[20] = '1-0-a'; + $form->hint = [ + [ + 'text' => 'Add', + 'format' => FORMAT_HTML, + ], + ]; + $form->status = \core_question\local\bank\question_version_status::QUESTION_STATUS_READY; return $form; diff --git a/question/type/calculatedsimple/tests/question_type_test.php b/question/type/calculatedsimple/tests/question_type_test.php index b1ebb1c7300..391a6d98098 100644 --- a/question/type/calculatedsimple/tests/question_type_test.php +++ b/question/type/calculatedsimple/tests/question_type_test.php @@ -90,7 +90,7 @@ final class question_type_test extends \advanced_testcase { $actualquestiondata = end($actualquestionsdata); foreach ($questiondata as $property => $value) { - if (!in_array($property, array('id', 'timemodified', 'timecreated', 'options', 'idnumber'))) { + if (!in_array($property, ['id', 'timemodified', 'timecreated', 'options', 'idnumber', 'hints'])) { $this->assertEquals($value, $actualquestiondata->$property); } } @@ -110,6 +110,11 @@ final class question_type_test extends \advanced_testcase { } } + $this->assertCount(1, $actualquestiondata->hints); + $hint = array_pop($actualquestiondata->hints); + $this->assertEquals($formdata->hint[0]['text'], $hint->hint); + $this->assertEquals($formdata->hint[0]['format'], $hint->hintformat); + $datasetloader = new qtype_calculated_dataset_loader($actualquestiondata->id); $this->assertEquals(10, $datasetloader->get_number_of_items()); diff --git a/question/type/ddimageortext/backup/moodle2/restore_qtype_ddimageortext_plugin.class.php b/question/type/ddimageortext/backup/moodle2/restore_qtype_ddimageortext_plugin.class.php index 4be59e8e834..09c646ac930 100644 --- a/question/type/ddimageortext/backup/moodle2/restore_qtype_ddimageortext_plugin.class.php +++ b/question/type/ddimageortext/backup/moodle2/restore_qtype_ddimageortext_plugin.class.php @@ -166,4 +166,29 @@ class restore_qtype_ddimageortext_plugin extends restore_qtype_plugin { return $contents; } + + #[\Override] + public static function convert_backup_to_questiondata(array $backupdata): \stdClass { + $questiondata = parent::convert_backup_to_questiondata($backupdata); + $questiondata->options->drags = array_map( + fn($drag) => (object) $drag, + $backupdata['plugin_qtype_ddimageortext_question']['drags']['drag'] ?? [], + ); + $questiondata->options->drops = array_map( + fn($drop) => (object) $drop, + $backupdata['plugin_qtype_ddimageortext_question']['drops']['drop'] ?? [], + ); + return $questiondata; + } + + #[\Override] + protected function define_excluded_identity_hash_fields(): array { + return [ + '/options/drags/id', + '/options/drags/questionid', + '/options/drops/id', + '/options/drops/questionid', + + ]; + } } diff --git a/question/type/ddimageortext/tests/helper.php b/question/type/ddimageortext/tests/helper.php index 8f0ee8810fd..13800b96bcf 100644 --- a/question/type/ddimageortext/tests/helper.php +++ b/question/type/ddimageortext/tests/helper.php @@ -205,12 +205,12 @@ class qtype_ddimageortext_test_helper extends question_test_helper { $fromform->penalty = '0.3333333'; $fromform->hint = array( array( - 'text' => '

Incorrect placements will be removed.

', + 'text' => '

1. Incorrect placements will be removed.

', 'format' => FORMAT_HTML, ), array( 'text' => '