avoid creating duplicates when restoring older backups (#197)

Backport of the same fix for the main branch (PR #196).

Author: PhilippImhof

CHANGES.md

@@ -1,5 +1,9 @@
 # Changelog
 
+### 5.3.6 (2025-04-29)
+
+- bugfix: avoid creating duplicates when restoring older backups
+
 ### 5.3.5 (2025-03-17)
 
 - improvement: avoid possible precision problem with ncr()

backup/moodle2/restore_qtype_formulas_plugin.class.php

@@ -44,7 +44,7 @@ protected function define_question_plugin_structure() {
         // section. For every question, we will have <formulas_answers></formulas_answers> to enclose
         // all parts. For each part, there will then be a <formulas_answer id="..."></formulas_answer>
         // section containing the various data fields, e. g. <placeholder> or <numbox> etc. These
-        // are the fields stored in the qtype_formulas_answers table, except for the partindex.
+        // are the fields stored in the qtype_formulas_answers table.
         $paths[] = new restore_path_element('formulas_answer', $this->get_pathfor('/formulas_answers/formulas_answer'));
 
         // Additionally, there will be <formulas id="..."></formulas> containing the custom data
@@ -188,8 +188,17 @@ public static function convert_backup_to_questiondata(array $backupdata): stdCla
         $questiondata = parent::convert_backup_to_questiondata($backupdata);
 
         // As our parts are backed up in a separate XML key rather than just "answers", the parent
-        // function did not add them to the questiondata.
-        foreach ($backupdata['plugin_qtype_formulas_question']['formulas_answers']['formulas_answer'] as $answer) {
+        // function did not add them to the questiondata. Old backups may lack the "answernotunique"
+        // key, in which case we add it here with the default value. Also, backups might miss the
+        // "partindex" key. In that case, we add the key and order the parts according to their appearance
+        // in the file.
+        foreach ($backupdata['plugin_qtype_formulas_question']['formulas_answers']['formulas_answer'] as $i => $answer) {
+            if (!key_exists('answernotunique', $answer)) {
+                $answer['answernotunique'] = '1';
+            }
+            if (!key_exists('partindex', $answer)) {
+                $answer['partindex'] = $i;
+            }
             $questiondata->options->answers[] = (object) $answer;
         }
 

tests/backup_restore_test.php

@@ -240,7 +240,7 @@ public function test_restore_quiz_with_same_stamp_questions(string $questionname
         // Create a question category.
         $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]);
 
-        // Create 2 quizzes with 2 questions multichoice.
+        // Create 2 quizzes with 2 questions.
         $quiz1 = $this->create_test_quiz($course1);
         $question1 = $questiongenerator->create_question('formulas', $questionname, ['category' => $cat->id]);
         quiz_add_quiz_question($question1->id, $quiz1, 0);
@@ -288,6 +288,90 @@ public function test_restore_quiz_with_same_stamp_questions(string $questionname
         $this->assertEquals($quiz2structure[2]->questionid, $quiz2structure[2]->questionid);
     }
 
+    /**
+     * Data provider.
+     *
+     * @return array
+     */
+    public static function provide_xml_keys_to_remove(): array {
+        return [
+            ['answernotunique'],
+            ['partindex'],
+        ];
+    }
+
+    /**
+     * Restore a quiz with a question where a field is missing in the backup.
+     *
+     * @param string $which the XML key to remove from the backup
+     *
+     * @dataProvider provide_xml_keys_to_remove
+     */
+    public function test_restore_quiz_if_field_is_missing_in_backup(string $which): void {
+        global $CFG, $USER;
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        // The changes introduced while fixing MDL-83541 are only present in Moodle 4.4 and newer. It
+        // does not make sense to perform this test with older versions.
+        if ($CFG->branch < 404) {
+            $this->markTestSkipped(
+                'Not testing detection of duplicates while restoring in Moodle versions prior to 4.4.',
+            );
+        }
+
+        // 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);
+        /** @var \core_question_generator $questiongenerator */
+        $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
+
+        // Create a question category.
+        $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]);
+
+        // Create a quiz with a multipart Formulas question.
+        $quiz = $this->create_test_quiz($course1);
+        $question = $questiongenerator->create_question('formulas', 'testmethodsinparts', ['category' => $cat->id]);
+        quiz_add_quiz_question($question->id, $quiz, 0);
+
+        // Backup quiz1.
+        $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();
+
+        // Delete requested entry from questions.xml file in the backup.
+        $xmlfile = $bc->get_plan()->get_basepath() . '/questions.xml';
+        $xml = file_get_contents($xmlfile);
+        $xml = preg_replace("=<$which>[^<]+</$which>=", '', $xml);
+        file_put_contents($xmlfile, $xml);
+
+        // Restore the (modified) 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 quiz1.
+        $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz');
+        $this->assertCount(2, $modules);
+        $quizstructure = \mod_quiz\question\bank\qbank_helper::get_question_structure(
+            $quiz->id,
+            \context_module::instance($quiz->cmid),
+        );
+        $restoredquiz = end($modules);
+        $restoredquizstructure = \mod_quiz\question\bank\qbank_helper::get_question_structure(
+            $restoredquiz->instance,
+            $restoredquiz->context,
+        );
+        $this->assertEquals($quizstructure[1]->questionid, $restoredquizstructure[1]->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
@@ -554,7 +638,7 @@ public function test_restore_quiz_with_same_stamp_questions_edited_hints(string
         // Create a question category.
         $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]);
 
-        // Create 2 questions multichoice.
+        // Create 2 questions.
         $quiz1 = $this->create_test_quiz($course1);
         $question1 = $questiongenerator->create_question('formulas', $questionname, ['category' => $cat->id]);
         quiz_add_quiz_question($question1->id, $quiz1, 0);
@@ -669,7 +753,7 @@ public function test_restore_quiz_with_same_stamp_questions_edited_options(strin
         // Create a question category.
         $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]);
 
-        // A quiz with 2 multichoice questions.
+        // A quiz with 2 questions.
         $quiz1 = $this->create_test_quiz($course1);
         $question1 = $questiongenerator->create_question('formulas', 'testsinglenum', ['category' => $cat->id]);
         quiz_add_quiz_question($question1->id, $quiz1, 0);
@@ -681,7 +765,10 @@ public function test_restore_quiz_with_same_stamp_questions_edited_options(strin
         $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]);
 
         // Change the options of question2 to be different to question1.
-        $optionfields = ['varsrandom', 'varsglobal', 'answernumbering', 'shownumcorrect', 'correctfeedback', 'partiallycorrectfeedback', 'incorrectfeedback'];
+        $optionfields = [
+            'varsrandom', 'varsglobal', 'answernumbering', 'shownumcorrect',
+            'correctfeedback', 'partiallycorrectfeedback', 'incorrectfeedback',
+        ];
         if (in_array($field, $optionfields)) {
             $DB->set_field('qtype_formulas_options', $field, $value, ['questionid' => $question2->id]);
         } else {

version.php

@@ -25,7 +25,7 @@
 defined('MOODLE_INTERNAL') || die();
 
 $plugin->component = 'qtype_formulas';
-$plugin->version = 2025031700;
+$plugin->version = 2025042900;
 
 $plugin->cron = 0;
 $plugin->requires = 2017111300;
@@ -35,6 +35,6 @@
     'qtype_multichoice' => 2015111600,
 );
 $plugin->supported = [39, 405];
-$plugin->release = '5.3.5 for Moodle 3.9 - 4.5';
+$plugin->release = '5.3.6 for Moodle 3.9 - 4.5';
 
 $plugin->maturity = MATURITY_STABLE;