Instruction for all question types in Attempt sheet screen

[HuongNV13]

Hi @timhunt, Please help me to review it

Including in this branch:

• Instruction for all question types in Attempt sheet screen
• Fix redirect not work when press Cancel button

The code was tested done by our tester

Thanks,

[HuongNV13]

I can see that Pull #6 already merged to master, so I rebased with latest master branch

[timhunt]

Thanks Huong. This looks good. Some discussion follows, but nothing to stop me merging this.

1) The renderer-wrangling you did to get the instructions to display is clever, and probably the best way. However, it is also a little scary. For the record, here is some discussion of other possible options and why they are less good:

• The crude alternative woudl be to use the standard question renderer, then apply regular-expression search-replace. That would be a hack, and probably harder to maintain.

• The other think we have done in the past is to add thing to the $options object like https://github.com/moodle/moodle/blob/8111abc33113e3c60f87aefea2cce28d25e5c9c3/question/engine/lib.php#L596. However, that requires core changes that we don't have time for now.

Also, this is just the first requirement like this we have here. We need to do things like displya options for Matching questions, etc. And your technique should extend to cover that.

So, overall, I think you chose the best way to handle this. Just outlining my thoughts here.

2) I am not sure it is good to test every single case in get_question_instruction_cases. In a sense, you have just written a unit tests for get_string. here ;-). Howver, I will not delay merging this. If we ever need to cahnge the instruction text, we can decide then whether to update the tests too, or remove some of the cases. (It is good to have some test that get_question_instruction works.)