Moodle
  1. Moodle
  2. MDL-17326

Error restoring question matchs. Could not recode answer

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9, 1.9.1, 1.9.2, 1.9.3
    • Fix Version/s: 1.6.9, 1.7.7, 1.8.8, 1.9.4
    • Component/s: Questions
    • Labels:
      None
    • Database:
      Any
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_16_STABLE, MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
    • Rank:
      30246

      Description

      I have fixed this issue in our moodle install so thought I'd post it here.

      getting error 'Could not recode answer in question_match_sub' in file question/type/match/questiontype.php method restore_recode_answer. The issue is that the $match_answer_id that's being used actually refers to the 'code' field in the 'question_match_sub' table. I added a line to get the correct id using the code value and assigned it to match_answer_id.

      There were also some other non fatal errors that I fixed. Just related to the variable $match_ans not being instantiated when called.

      I also found in the backup method of the match questiontype that the details of the 'question_match' table aren't written to the backup file at all. Instead
      details from 'question_match_sub' are added as 'MATCH' items when in fact they should be 'MATCH_SUB'.

      If we just corrected the name we'd break from the standard moodle backup api that has this bug in it. So for now I just added the details in the element
      'MATCH_DEFINITIONS' for want of a better name.

      The bug is important because it may make a backup fail if it includes a match questiontype

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Thanks Colin, but can you attach a patch? Thanks.

          Show
          Tim Hunt added a comment - Thanks Colin, but can you attach a patch? Thanks.
          Hide
          Colin Chambers added a comment -

          Sure,

          Not quite sure how to create patches (kinda rushed so I'll figure that out later). So for now I'll paste the methods as they now stand. Our bugzilla number for this bug is 6750

          /*

          • Backup the data in the question
            *
          • This is used in question/backuplib.php
            */
            function backup($bf,$preferences,$question,$level=6) {

          $status = true;

          // ou-specific begins export standard moodle backups bug #6750
          /*

          • Moodle core forgets to backup the 'question_match' table. We need to
          • add it. The 'question_match_sub' is backed up in it's place. To fit
          • in with this the records for 'question_match' will be included under
          • the tag 'MATCH_DEFINITIONS' so as not to break the standard moodle
          • backup.
            */
            $records = get_records('question_match', 'question', $question, 'id ASC');

          if(function_exists('ou_backup_writeRecordsToFile'))

          { $status = ou_backup_writeRecordsToFile($bf, $records, 6, 'MATCH_DEFINITIONS', 'MATCH_DEFINITION'); }

          // ou-specific ends export standard moodle backups

          $records = get_records('question_match_sub', 'question', $question, 'id ASC');
          //If there are matchs
          if ($records) {
          $status = fwrite ($bf,start_tag("MATCHS",6,true));
          //Iterate over each match
          foreach ($records as $match)

          { $status = fwrite ($bf,start_tag("MATCH",7,true)); //Print match contents fwrite ($bf,full_tag("ID",8,false,$match->id)); fwrite ($bf,full_tag("CODE",8,false,$match->code)); fwrite ($bf,full_tag("QUESTIONTEXT",8,false,$match->questiontext)); fwrite ($bf,full_tag("ANSWERTEXT",8,false,$match->answertext)); $status = fwrite ($bf,end_tag("MATCH",7,true)); }

          $status = fwrite ($bf,end_tag("MATCHS",6,true));
          }
          return $status;
          }

          /*

          • Restores the data in the question
            *
          • This is used in question/restorelib.php
            */
            function restore($old_question_id,$new_question_id,$info,$restore) {

          $status = true;

          //Get the matchs array
          $matchs = $info'#'['MATCHS']['0']'#'['MATCH'];

          //We have to build the subquestions field (a list of match_sub id)
          $subquestions_field = "";
          $in_first = true;

          //Iterate over matchs
          for($i = 0; $i < sizeof($matchs); $i++) {
          $mat_info = $matchs[$i];

          //We'll need this later!!
          $oldid = backup_todb($mat_info'#'['ID']['0']'#');

          //Now, build the question_match_SUB record structure
          $match_sub = new stdClass;
          $match_sub->question = $new_question_id;
          $match_sub->code = isset($mat_info'#'['CODE']['0']'#')?backup_todb($mat_info'#'['CODE']['0']'#'):'';
          if (!$match_sub->code)

          { $match_sub->code = $oldid; }

          $match_sub->questiontext = backup_todb($mat_info'#'['QUESTIONTEXT']['0']'#');
          $match_sub->answertext = backup_todb($mat_info'#'['ANSWERTEXT']['0']'#');

          //The structure is equal to the db, so insert the question_match_sub
          $newid = insert_record ("question_match_sub",$match_sub);

          //Do some output
          if (($i+1) % 50 == 0) {
          if (!defined('RESTORE_SILENTLY')) {
          echo ".";
          if (($i+1) % 1000 == 0)

          { echo "<br />"; }
          }
          backup_flush(300);
          }

          if ($newid) {
          //We have the newid, update backup_ids
          backup_putid($restore->backup_unique_code,"question_match_sub",$oldid,
          $newid);
          //We have a new match_sub, append it to subquestions_field
          if ($in_first) { $subquestions_field .= $newid; $in_first = false; } else { $subquestions_field .= ",".$newid; }
          } else { $status = false; }
          }

          //We have created every match_sub, now create the match

          // ou-specific begins export standard moodle backups bug #6750
          /*
          * Moodle core forgets to backup the 'question_match' table. We need to
          * add it. The 'question_match_sub' is backed up in it's place. To fit
          * in with this the records for 'question_match' will be included under
          * the tag 'MATCH_DEFINITIONS' so as not to break the standard moodle
          * backup.
          */
          // Get the match definitions from the backup file to the db.
          $records = null;
          if(isset($info'#'['MATCH_DEFINITIONS']) && isset($info'#'['MATCH_DEFINITIONS']['0']'#'['MATCH_DEFINITION'])){ $records = $info['#']['MATCH_DEFINITIONS']['0']['#']['MATCH_DEFINITION']; }
          if(isset($records) && function_exists('ou_restore_getRecordsFromFileToDB')){ $status = ou_restore_getRecordsFromFileToDB($restore, $records, $new_question_id, 'question_match', 'question'); }

          /* deleted
          *
          $match = new stdClass;
          $match->question = $new_question_id;
          $match->subquestions = $subquestions_field;

          //The structure is equal to the db, so insert the question_match_sub
          $newid = insert_record ("question_match",$match);

          if (!$newid) { $status = false; }
          */
          // ou-specific ends export standard moodle backups

          return $status;
          }


          function restore_recode_answer($state, $restore) {

          //The answer is a comma separated list of hypen separated math_subs (for question and answer)
          $answer_field = "";
          $in_first = true;
          $tok = strtok($state->answer,",");
          //ou-specific begins inc backup bug #6750
          $match_ans = null;
          //ou-specific begins inc backup
          while ($tok) {
          //Extract the match_sub for the question and the answer
          $exploded = explode("-",$tok);

          $match_question_id = $exploded[0];
          $match_answer_id = $exploded[1];

          //Get the match_sub from backup_ids (for the question)
          if (!$match_que = backup_getid($restore->backup_unique_code,"question_match_sub",$match_question_id)) { echo 'Could not recode question in question_match_sub '.$match_question_id.'<br />'; }
          //Get the match_sub from backup_ids (for the answer)
          if ($match_answer_id) { // only recode answer if not 0, not answered yet
          //ou-specific begins inc backup bug #6750
          $match_answer_id = get_field('question_match_sub', 'id', 'code', $match_answer_id);
          //ou-specific ends inc backup
          if (!$match_ans = backup_getid($restore->backup_unique_code,"question_match_sub",$match_answer_id)) { echo 'Could not recode answer in question_match_sub '.$match_answer_id.'<br />'; }
          }

          if ($match_que) {
          //If the question hasn't response, it must be 0
          if (!$match_ans and $match_answer_id == 0) { $match_ans->new_id = 0; }

          if ($in_first) { $answer_field .= $match_que->new_id."-".$match_ans->new_id; $in_first = false; } else { $answer_field .= ",".$match_que->new_id."-".$match_ans->new_id; }
          }
          //check for next
          $tok = strtok(",");
          }
          return $answer_field;
          }


          To save having to manually write out the db field to xml field translations I created helper methods for backup and restore. These methods are in the files ou_restoreling.php and ou_backuplib.php respectively with the files included in backuplib.php and restorelib.php.
          /*
          * Write the fields from an object to the backup file
          * @param object $bf backup file passed in
          * @param int $indent level of indentation
          * @param array array of records to backup
          * @param string $name name to call the xml elements created
          */
          function ou_backup_writeRecordsToFile($bf, $records, $indent=0, $element_plural, $element_single){

          $status = true;

          if (!$records) { return $status; }

          //If there are records
          $status = fwrite ($bf,start_tag($element_plural,$indent,true));

          $indent++;
          //Iterate over each record
          foreach ($records as $record) {
          $status = fwrite ($bf,start_tag($element_single,$indent,true));
          //Print record contents
          foreach($record as $field => $value){ fwrite ($bf,full_tag(strtoupper($field),$indent+1,false,$value)); }
          $status = fwrite ($bf,end_tag($element_single,$indent,true));
          }
          $indent--;
          $status = fwrite ($bf,end_tag($element_plural,$indent,true));

          return $status;
          }

          /*
          * Get item records from the backup xml file and save to the database
          * @param object $preferences preferences object that controls the restore,
          * @param array $records set of item records ,
          * @param int $new_id,
          * @param string $table table to insert records to.
          * @param string $id_field name of the field to assign $new_id to
          */
          function ou_restore_getRecordsFromFileToDB($preferences, $records, $new_id, $table, $id_field) {

          $status = true;
          $field_name = null;

          //Iterate over records
          for($i = 0; $i < sizeof($records); $i++) {
          if(empty($records[$i]'#')){ continue; }

          $record = $records[$i]'#';
          //We'll need this later!!
          $oldid = backup_todb($record['ID']['0']'#');

          //Now, build the record structure
          $db_record = new stdClass;

          //Iterate over each record
          foreach ($record as $field => $value) { $field_name = strtolower($field); $db_record->$field_name = backup_todb($record[$field]['0']['#']); }

          //$id_field will be set within the previous loop so set it correctly here.
          $db_record->$id_field = $new_id;

          //The structure is equal to the db, so insert the record
          $newid = insert_record ($table,$db_record);

          //Do some output
          if (($i+1) % 50 == 0) {
          if (!defined('RESTORE_SILENTLY')) {
          echo ".";
          if (($i+1) % 1000 == 0) { echo "<br />"; }

          }
          backup_flush(300);
          }

          if ($newid)

          { //We have the newid, update backup_ids backup_putid($preferences->backup_unique_code,$table,$oldid, $newid); }

          else

          { $status = false; }

          }

          return $status;
          }

          Show
          Colin Chambers added a comment - Sure, Not quite sure how to create patches (kinda rushed so I'll figure that out later). So for now I'll paste the methods as they now stand. Our bugzilla number for this bug is 6750 /* Backup the data in the question * This is used in question/backuplib.php */ function backup($bf,$preferences,$question,$level=6) { $status = true; // ou-specific begins export standard moodle backups bug #6750 /* Moodle core forgets to backup the 'question_match' table. We need to add it. The 'question_match_sub' is backed up in it's place. To fit in with this the records for 'question_match' will be included under the tag 'MATCH_DEFINITIONS' so as not to break the standard moodle backup. */ $records = get_records('question_match', 'question', $question, 'id ASC'); if(function_exists('ou_backup_writeRecordsToFile')) { $status = ou_backup_writeRecordsToFile($bf, $records, 6, 'MATCH_DEFINITIONS', 'MATCH_DEFINITION'); } // ou-specific ends export standard moodle backups $records = get_records('question_match_sub', 'question', $question, 'id ASC'); //If there are matchs if ($records) { $status = fwrite ($bf,start_tag("MATCHS",6,true)); //Iterate over each match foreach ($records as $match) { $status = fwrite ($bf,start_tag("MATCH",7,true)); //Print match contents fwrite ($bf,full_tag("ID",8,false,$match->id)); fwrite ($bf,full_tag("CODE",8,false,$match->code)); fwrite ($bf,full_tag("QUESTIONTEXT",8,false,$match->questiontext)); fwrite ($bf,full_tag("ANSWERTEXT",8,false,$match->answertext)); $status = fwrite ($bf,end_tag("MATCH",7,true)); } $status = fwrite ($bf,end_tag("MATCHS",6,true)); } return $status; } /* Restores the data in the question * This is used in question/restorelib.php */ function restore($old_question_id,$new_question_id,$info,$restore) { $status = true; //Get the matchs array $matchs = $info '#' ['MATCHS'] ['0'] '#' ['MATCH'] ; //We have to build the subquestions field (a list of match_sub id) $subquestions_field = ""; $in_first = true; //Iterate over matchs for($i = 0; $i < sizeof($matchs); $i++) { $mat_info = $matchs [$i] ; //We'll need this later!! $oldid = backup_todb($mat_info '#' ['ID'] ['0'] '#' ); //Now, build the question_match_SUB record structure $match_sub = new stdClass; $match_sub->question = $new_question_id; $match_sub->code = isset($mat_info '#' ['CODE'] ['0'] '#' )?backup_todb($mat_info '#' ['CODE'] ['0'] '#' ):''; if (!$match_sub->code) { $match_sub->code = $oldid; } $match_sub->questiontext = backup_todb($mat_info '#' ['QUESTIONTEXT'] ['0'] '#' ); $match_sub->answertext = backup_todb($mat_info '#' ['ANSWERTEXT'] ['0'] '#' ); //The structure is equal to the db, so insert the question_match_sub $newid = insert_record ("question_match_sub",$match_sub); //Do some output if (($i+1) % 50 == 0) { if (!defined('RESTORE_SILENTLY')) { echo "."; if (($i+1) % 1000 == 0) { echo "<br />"; } } backup_flush(300); } if ($newid) { //We have the newid, update backup_ids backup_putid($restore->backup_unique_code,"question_match_sub",$oldid, $newid); //We have a new match_sub, append it to subquestions_field if ($in_first) { $subquestions_field .= $newid; $in_first = false; } else { $subquestions_field .= ",".$newid; } } else { $status = false; } } //We have created every match_sub, now create the match // ou-specific begins export standard moodle backups bug #6750 /* * Moodle core forgets to backup the 'question_match' table. We need to * add it. The 'question_match_sub' is backed up in it's place. To fit * in with this the records for 'question_match' will be included under * the tag 'MATCH_DEFINITIONS' so as not to break the standard moodle * backup. */ // Get the match definitions from the backup file to the db. $records = null; if(isset($info '#' ['MATCH_DEFINITIONS'] ) && isset($info '#' ['MATCH_DEFINITIONS'] ['0'] '#' ['MATCH_DEFINITION'] )){ $records = $info['#']['MATCH_DEFINITIONS']['0']['#']['MATCH_DEFINITION']; } if(isset($records) && function_exists('ou_restore_getRecordsFromFileToDB')){ $status = ou_restore_getRecordsFromFileToDB($restore, $records, $new_question_id, 'question_match', 'question'); } /* deleted * $match = new stdClass; $match->question = $new_question_id; $match->subquestions = $subquestions_field; //The structure is equal to the db, so insert the question_match_sub $newid = insert_record ("question_match",$match); if (!$newid) { $status = false; } */ // ou-specific ends export standard moodle backups return $status; } function restore_recode_answer($state, $restore) { //The answer is a comma separated list of hypen separated math_subs (for question and answer) $answer_field = ""; $in_first = true; $tok = strtok($state->answer,","); //ou-specific begins inc backup bug #6750 $match_ans = null; //ou-specific begins inc backup while ($tok) { //Extract the match_sub for the question and the answer $exploded = explode("-",$tok); $match_question_id = $exploded [0] ; $match_answer_id = $exploded [1] ; //Get the match_sub from backup_ids (for the question) if (!$match_que = backup_getid($restore->backup_unique_code,"question_match_sub",$match_question_id)) { echo 'Could not recode question in question_match_sub '.$match_question_id.'<br />'; } //Get the match_sub from backup_ids (for the answer) if ($match_answer_id) { // only recode answer if not 0, not answered yet //ou-specific begins inc backup bug #6750 $match_answer_id = get_field('question_match_sub', 'id', 'code', $match_answer_id); //ou-specific ends inc backup if (!$match_ans = backup_getid($restore->backup_unique_code,"question_match_sub",$match_answer_id)) { echo 'Could not recode answer in question_match_sub '.$match_answer_id.'<br />'; } } if ($match_que) { //If the question hasn't response, it must be 0 if (!$match_ans and $match_answer_id == 0) { $match_ans->new_id = 0; } if ($in_first) { $answer_field .= $match_que->new_id."-".$match_ans->new_id; $in_first = false; } else { $answer_field .= ",".$match_que->new_id."-".$match_ans->new_id; } } //check for next $tok = strtok(","); } return $answer_field; } To save having to manually write out the db field to xml field translations I created helper methods for backup and restore. These methods are in the files ou_restoreling.php and ou_backuplib.php respectively with the files included in backuplib.php and restorelib.php. /* * Write the fields from an object to the backup file * @param object $bf backup file passed in * @param int $indent level of indentation * @param array array of records to backup * @param string $name name to call the xml elements created */ function ou_backup_writeRecordsToFile($bf, $records, $indent=0, $element_plural, $element_single){ $status = true; if (!$records) { return $status; } //If there are records $status = fwrite ($bf,start_tag($element_plural,$indent,true)); $indent++; //Iterate over each record foreach ($records as $record) { $status = fwrite ($bf,start_tag($element_single,$indent,true)); //Print record contents foreach($record as $field => $value){ fwrite ($bf,full_tag(strtoupper($field),$indent+1,false,$value)); } $status = fwrite ($bf,end_tag($element_single,$indent,true)); } $indent--; $status = fwrite ($bf,end_tag($element_plural,$indent,true)); return $status; } /* * Get item records from the backup xml file and save to the database * @param object $preferences preferences object that controls the restore, * @param array $records set of item records , * @param int $new_id, * @param string $table table to insert records to. * @param string $id_field name of the field to assign $new_id to */ function ou_restore_getRecordsFromFileToDB($preferences, $records, $new_id, $table, $id_field) { $status = true; $field_name = null; //Iterate over records for($i = 0; $i < sizeof($records); $i++) { if(empty($records [$i] '#' )){ continue; } $record = $records [$i] '#' ; //We'll need this later!! $oldid = backup_todb($record ['ID'] ['0'] '#' ); //Now, build the record structure $db_record = new stdClass; //Iterate over each record foreach ($record as $field => $value) { $field_name = strtolower($field); $db_record->$field_name = backup_todb($record[$field]['0']['#']); } //$id_field will be set within the previous loop so set it correctly here. $db_record->$id_field = $new_id; //The structure is equal to the db, so insert the record $newid = insert_record ($table,$db_record); //Do some output if (($i+1) % 50 == 0) { if (!defined('RESTORE_SILENTLY')) { echo "."; if (($i+1) % 1000 == 0) { echo "<br />"; } } backup_flush(300); } if ($newid) { //We have the newid, update backup_ids backup_putid($preferences->backup_unique_code,$table,$oldid, $newid); } else { $status = false; } } return $status; }
          Hide
          Tim Hunt added a comment -

          In Eclipse, Window -> Show view -> Other ... -> Team -> Synchronise (If you don't already have it open.)

          The select some files in that view, and do create patch from the context menu.

          For other options see http://docs.moodle.org/en/Development:How_to_create_a_patch.

          Show
          Tim Hunt added a comment - In Eclipse, Window -> Show view -> Other ... -> Team -> Synchronise (If you don't already have it open.) The select some files in that view, and do create patch from the context menu. For other options see http://docs.moodle.org/en/Development:How_to_create_a_patch .
          Hide
          Tim Hunt added a comment -

          I don't really like your fix. To start with you don't really need MATCH_DEFINITIONS and MATCH_DEFINITION, because there is only one MATCH_DEFINITION for each matching questions. Also, MATCH_DEFINITION is long, and unrelated to any other naming convention in the quiz. MATCH_OPTIONS is probably the least bad, since MATCH is taken. (MATCH would be consistent with how every other question type that has data like this backs it up.) MATCH_OPTIONS is good because elsewhere in the code, when this data is loaded into memory, it gets stored in $question->options.

          Also, code like ou_backup_writeRecordsToFile/ou_restore_getRecordsFromFileToDB is nice in abstract, and probably backup and restore should have had something like that from the start, but unfortunately, in core at the moment, consistency is more important.

          So I will probably reimplement your fix in the next couple of days, but at least it is very clear what I have to do. Sorry or thanks, whichever you feel is more appropriate.

          Show
          Tim Hunt added a comment - I don't really like your fix. To start with you don't really need MATCH_DEFINITIONS and MATCH_DEFINITION, because there is only one MATCH_DEFINITION for each matching questions. Also, MATCH_DEFINITION is long, and unrelated to any other naming convention in the quiz. MATCH_OPTIONS is probably the least bad, since MATCH is taken. (MATCH would be consistent with how every other question type that has data like this backs it up.) MATCH_OPTIONS is good because elsewhere in the code, when this data is loaded into memory, it gets stored in $question->options. Also, code like ou_backup_writeRecordsToFile/ou_restore_getRecordsFromFileToDB is nice in abstract, and probably backup and restore should have had something like that from the start, but unfortunately, in core at the moment, consistency is more important. So I will probably reimplement your fix in the next couple of days, but at least it is very clear what I have to do. Sorry or thanks, whichever you feel is more appropriate.
          Hide
          Colin Chambers added a comment -

          No problems. Match options sounds better. I just didn't know what convention to follow and I didn't have much time. The method abstraction is something I'm working on when I get time. I'm keeping it separate from core code as you can see because it's new. I didn't expect core to want it but didn't have time to translate.

          Atleast you could make sense of it.

          col

          Show
          Colin Chambers added a comment - No problems. Match options sounds better. I just didn't know what convention to follow and I didn't have much time. The method abstraction is something I'm working on when I get time. I'm keeping it separate from core code as you can see because it's new. I didn't expect core to want it but didn't have time to translate. Atleast you could make sense of it. col
          Hide
          Tim Hunt added a comment -

          Fixed.

          Show
          Tim Hunt added a comment - Fixed.
          Hide
          Kevin Brake added a comment -

          I have encountered an issue when restoring a backup of a course that includes the use of the [[answertemplate]] it does not include the [[answertemplate]] contents into the restored course. More information here http://moodle.org/mod/forum/discuss.php?d=172189.

          I am not sure if the code you have provided needs to replace the existing backuplib.php or needs to be added to the existing backuplib.php?

          Thanks,

          Kevin Brake
          eLearningShow.com

          Show
          Kevin Brake added a comment - I have encountered an issue when restoring a backup of a course that includes the use of the [ [answertemplate] ] it does not include the [ [answertemplate] ] contents into the restored course. More information here http://moodle.org/mod/forum/discuss.php?d=172189 . I am not sure if the code you have provided needs to replace the existing backuplib.php or needs to be added to the existing backuplib.php? Thanks, Kevin Brake eLearningShow.com

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: