penalty and Hints donot work with XML export for Cloze questions

Pierre Pichet added a comment - 04/Oct/11 10:13 PM - edited The problem is located in the format.php file the froeach was modifying the main $question foreach ($question->options->questions as $question) { replacing by something else like $subquestion solve the problem format.php line 1240 {noformat} case 'multianswer': $acount = 1; foreach ($question->options->questions as $subquestion) { $thispattern = " {#".$acount."} "; $thisreplace = $subquestion->questiontext; $expout = preg_replace(" $thispattern ", $thisreplace, $expout); $acount++; } break;

Tim Hunt added a comment - 04/Oct/11 11:53 PM Now ready to integrate.

Tim Hunt added a comment - 04/Oct/11 11:53 PM Pierre was right about the basic problem, but it was important to also create some unit tests so that we can easily verify that this is still working in future.

Pierre Pichet added a comment - 05/Oct/11 1:15 AM - edited Commit https://github.com/ppichet/moodle/commit/292819c3441f14002f1131435415a866e0d4f27a tested on import-export and hints are saved. Oups I am somehow late (lunch time...)

Pierre Pichet added a comment - 05/Oct/11 4:38 AM "but it was important to also create some unit tests so that we can easily verify that this is still working in future." I agree completly to your statement and these tests that you take great care to build will effectively play an important role in Moodle actual and future stability.

Eloy Lafuente (stronk7) added a comment - 05/Oct/11 5:26 AM Was going to integrate this when I detected one Q with penalty = 0.5 in xml being imported as penalty = 0.333 (in edit Q page).

Tim Hunt added a comment - 05/Oct/11 5:30 AM OK, commit amended so that: 1. The unit test uses a non-default value of penalty, 2. and then actually checks the imported value. 3. Then the code was fixed so that it actually works. Hopefully this can be integrated now.

Eloy Lafuente (stronk7) added a comment - 05/Oct/11 5:43 AM Integrated, thanks!

Eloy Lafuente (stronk7) added a comment - 05/Oct/11 5:44 AM While reviewing and integrating export / import and unittests have been checked, so passing this. Thanks!

Pierre Pichet added a comment - 05/Oct/11 10:51 AM - edited Tim Just as a proof that your work to build "some unit tests so that we can easily verify that this is still working in future" is the thing to do. I look at the history of those code lines. This brink us back to http://cvs.moodle.org/moodle/question/format/xml/format.php?revision=1.5.2.3&view=markup where the cloze case was added case MATCH: foreach($question->options->subquestions as $subquestion) { $expout .= "<subquestion>\n"; $expout .= $this->writetext( $subquestion->questiontext ); $expout .= "<answer>".$this->writetext( $subquestion->answertext )."</answer>\n"; $expout .= "</subquestion>\n"; } break; case DESCRIPTION: // nothing more to do for theis type break; case MULTIANSWER: $a_count=1; foreach($question->options->questions as $question) { $thispattern = addslashes("{#".$a_count."}"); $thisreplace = $question->questiontext; $expout=ereg_replace($thispattern, $thisreplace, $expout ); $a_count++; } break; default: $expout .= "<!-- Question type is unknown or not supported (Type=$question->qtype) -->\n"; } // close the question tag $expout .= "</question>\n"; The problem was not seen as the cloze code was the last questiontype before closing the output. The irony is that the preceeding case i.e. MATCH was correctly written. This bring us back to Revision 1.5.2.3 - (view) (download) (annotate) - [select for diffs] Fri Jun 2 17:44:13 2006 WST (5 years, 4 months ago) by thepurpleblob Changes since 1.5.2.2: +113 -18 lines Diff to previous 1.5.2.2 , to branch point 1.5 Added CLOZE question support. Thanks to Joseph Rezeau for this one! However as things got more complex, new paramaters were added to all questions. The first were tags and they were added at the output end So default: // try support by optional plugin if (!$data = $this->try_exporting_using_qtypes( $question->qtype, $question )) { echo $OUTPUT->notification( get_string( 'unsupportedexport','qformat_xml',$QTYPES[$question->qtype]->local_name() ) ); } $expout .= $data; } // Write the question tags. if (!empty($CFG->usetags)) { require_once($CFG->dirroot.'/tag/lib.php'); $tags = tag_get_tags_array('question', $question->id); if (!empty($tags)) { $expout .= " <tags>\n"; foreach ($tags as $tag) { $expout .= " <tag>" . $this->writetext($tag, 0, true) . "</tag>\n"; } $expout .= " </tags>\n"; } } // close the question tag $expout .= "</question>\n"; return $expout; } you know well the author Revision 1.74 - (view) (download) (annotate) - [select for diffs] Fri Nov 19 20:33:48 2010 WST (10 months, 2 weeks ago) by tjhunt Branch: MAIN Changes since 1.73: +30 -11 lines Diff to previous 1.73 question XML import/export MDL-18916 should include question tags. This go unnoticed as few peoples use tags for cloze if at all. Adding hints to cloze create the bug. The real lesson is that Moodle code is growing in complexity and that your test policy is the good thing to do. This policy is another aspect of your professionalism as a computer code expert. So continue

Eloy Lafuente (stronk7) added a comment - 07/Oct/11 6:34 AM git repositories have been updated with your awesome changes, thanks! Closing.