Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 2.1.1, 2.2
-
Fix Version/s: 2.1.2
-
Component/s: Questions
-
Labels:None
-
Testing Instructions:
-
Affected Branches:MOODLE_21_STABLE, MOODLE_22_STABLE
-
Fixed Branches:MOODLE_21_STABLE
-
Pull from Repository:
-
Pull Master Branch:
-
Pull Master Diff URL:
Description
refer MDL-29216 for details.
Create a cloze question with Penalty and Hints
Try to export the question as MOODLE XML.
Exported file doesn't contain any <hint> tags.
Gliffy Diagrams
Attachments
Issue Links
- is a regression caused by
-
-
- Closed
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Links Hierarchy
Now ready to integrate.
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.
Commit
https://github.com/ppichet/moodle/commit/292819c3441f14002f1131435415a866e0d4f27a
tested on import-export and hints are saved.
Oups I am somehow late (lunch time...)
"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. 
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).
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.
Integrated, thanks!
While reviewing and integrating export / import and unittests have been checked, so passing this. Thanks!
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
git repositories have been updated with your awesome changes, thanks! Closing.
The problem is located in the format.php file
{ replacing by something else like $subquestion solve the problem format.php line 1240 {noformat}the froeach was modifying the main $question
foreach ($question->options->questions as $question)
case 'multianswer':
{#".$acount."}$acount = 1;
foreach ($question->options->questions as $subquestion) {
$thispattern = "
";
$thisreplace = $subquestion->questiontext;
$expout = preg_replace("$thispattern", $thisreplace, $expout);
$acount++;
}
break;