Multianswer question import format is broken in Moodle 2.2 and master

I exported a cloze question using moodle xml format and reimported in master and did not have any problem.
Using cloze specific format,things were OK but this format does not handle multilines cloze question.

However the bug appears as described in 2,2.

Tim,
On further tests the Cloze import does not work on master.
I probably use the gift import
I obtain (as in 2,2)
Fatal error: Call to undefined function qtype_multianswer_extract_question() in D:\moodle2\server\moodle\M22\moodle\question\format\multianswer\format.php on line 53

So I add
require_once($CFG->dirroot . '/question/type/multianswer/questiontype.php'); in multianswer/format.php
Questions import correctly

As function qtype_multianswer_extract_question() is not a function internal to multianswer class but
a static function inside the multianswer/questiontype.php file, do you think that this is the best way to solve this bug ?

If so, I will prepare the gits.

By the way

Strict Standards: Only variables should be passed by reference in D:\moodle2\server\moodle\moodle_v\question\type\multianswer\renderer.php on line 282

$rightanswer = $subq->answers[$order[reset($subq->get_correct_response())]];

What is your solution to this ?

Hello Pierre,
rather than adding require_once($CFG->dirroot . '/question/type/multianswer/questiontype.php');
at the begining, you may consider adding
question_bank::get_qtype('multianswer');
in the readquestions method
If you study the get_qtype method you will see it will have the desired effect and this is how it is done in the xml import format.
But this is just my understanding so it's better to wait for Tim comment.

However in the moodle xml this is done in

public function import_multianswer($question) {
question_bank::get_qtype('multianswer');

not in the readquestions() function.

But the code flow is different as multianswer import format is a unique question import.
This could explain why it has not be used often.

I just worry that using the more general question_bank::get_qtype will as a side effect by loading multianswer/questiontype.php, load the desired qtype_multianswer_extract_question function.

But effectively the two solutions are equivalent and the question_bank::get_qtype() respect the new more general syntax.

The fix was easy. Making a unit test took a little longer.

Jean-Michel, are you able to peer-review this?

Hello Tim,
I will be glad to peer-review.

• I spotted a typo in the comment on line 43.
• Is there a norm for the number of spaces between @subpackage, @copyright, @license and the following word ? It seems to vary from file to file in Moodle.
• I am always unsure if FORMAT_MOODLE is a good choice for questiontext and generalfeedback. Of course it's the most compatible with previous behaviour, but users don't get the editor when they try to edit the question and they don't always know the trick (change, save, re-open).
• Unless I miss something the changes to the questiontype regex seems unrelated to this issue.
  Has it something to do with other problems in multianswer qtype ?
  I am unsure if peer reviewing include trying the code myself, runing codechecker or phpunit tests ? if yes I will only be able to do it in a few hours as I am away form my development computer and posting this from another laptop.

1. Oops! fixed.

2. I like to make them line up like this, but that is personal preference.

3. I think it is a good choice for import formats that are close to plain text.

4. Actually, that was essential to make the unit tests pass. Note how, in the test input, the shortanswer question is split over several lines.

Also, we should not have the tests subfolder in Moodle 2.2.

Oops I missed the test subfolder in Moodle 2.2
Also I missed the problem of shortanswer split over several lines reading too quickly the sample question, so made wrong deductions not seeng why the s option could be used for.
Not very good for a first peer-review. Sorry !

Thanks for the patches and phpunit tests Tim
Integrated into 23 and master. The fix is in 22 also.

jftr Jean-Michel Vedrine, that was a very proper peer-review imo . It brought about the thoughts and concerns that were needed.

Works Great,

Thanks for fixing this Tim.

I'm so proud...of you, many thanks!

http://youtu.be/n64CdfDRnZY

Closing as fixed, ciao