Bug in the edit form if you try to blank the combined feedback

[timhunt]

To reproduce:

1. Rename the attached file to .xml (why won't github let you attach a .xml file?!) wordselect-example.txt
2. Import it into a Moodle question bank.
3. Preview the question.
4. Check all the review options are on.
5. Get the question right.
6. Note there is no feedback, when you expect there to be.
7. To try to diagnose, find the question in the question bank, and edit it.
8. Note that the Combined feedback -> For any correct response says "Your answer is correct."
9. Get really, really confused.
10. Look at the source of the XML file you imported. Ah! in fact the Correct feedback is blank! That explains it.

So, in summary:

Expected result: if the question has not combined feedback, then when you edit the question, the combined feedback box should be empty.

Actual result: the combined feedback box contains the default text.

Suggested fix: in qtype_wordselect_edit_form::data_preprocessing you should have a call to $question = $this->data_preprocessing_combined_feedback($question, true); (like in qtype_multichoice, etc.)

However: if you do that, you get Notice: Undefined property: stdClass::$shownumcorrect in .../question/type/edit_question_form.php

Since the fix is not completely trivial, I can't send you a pull request now, but I thought I would log the issue.

[marcusgreen]

Thanks for reporting this Tim. Looks like there should be a behat test for this.

[marcusgreen]

Have reproduced what you have seen. Will look at fixes as soon as I can.

[marcusgreen]

Have you tried $question = $this->data_preprocessing_combined_feedback($question); The second parameter defaults to false. I have just tried it with immediate and interactive behaviours and based on about 2 minutes experimentation it seems to work.

[marcusgreen]

Hmm the dbtable has no shownumcorrect field....

[timhunt]

I was suggesting true because the call in definition_inner had true, and they should match. It coudl be that false is right in both places (at least until the DB is changed.)

[marcusgreen]

What does shownumcorrect actually control? I would guess it controls the showing of how many responses are correct at runtime, but that has seemed to work for me, but perhaps it has just always defaulted to being on.

[timhunt]

O.K. I think I have reverse engineered this, to remind myself how it is supposed to work. The history is that getting the core question engine to handle the concept of 'number of parts of the question correct' rather than making each qtype do all the work, was a bit of an afterthought, but once you understand it, it means the core system implements in consistently for all qtypes and does some of the work for you.

O.K. so the only way to signal to the system that you want to buy into this is just to implement the get_num_parts_right method in your question class, and have it return something other than [null, null].

If you do this, you are strongly encouraged to add a setting to your question DB table, and on the editing for so that for any given question, the teacher can decide whether the student should be told this information in the feedback. If you have the setting, you should add both the setting under combined feedback (for when the question is finished - pass true to the methods we discussed above) and also a setting for each hint (pass true to the add hints methods).

Then, to make it work, in your question_type::make_hint method, you need to create question_hint_with_parts instead of question_hint, and in initialise_question, pass true to initialise_combined_feedback.

And have some unit/Behat tests to verify it works.

In core, qtype_match does everything needed.

Wow! we really need a docs page about this, or at least a section on https://docs.moodle.org/dev/Question_types.

Pester me on Telegram tomorrow if this is still incomprehensible.

[timhunt]

I am taking this, building on the above suggestion, which Mahmoud verified yesterday.

Also, while fixing this, I have dealt with updating the Behat tests for this change: https://github.com/moodle/moodle/blob/e04a73ccc06e18d8d3b3661f8f9bc16911747830/question/type/upgrade.txt#L5