Hardcoded strings in some question imports formats

examview line 227
missing word lines 74 82 110
learnwise line 124
xhtml line 147
Also it would be good to use the same string in all formats when a question type is not handled.

Are you able to fix this Jean-Michel. I agree with everything you say about what needs to be changed.

Hello Tim,
Yes, this was my intention to fix this, but better let the "Big Boss" of quiz and question decide So you can happily assign this issue to me.

You are welcome to assign issues to yourself, if you are planning to work on them in the near future. Just add me as a watcher first, so I know what is going on.

Looking good, although there will be a few things to fix when you run codechecker.

Oops! Note that MDL-37068 is waiting for integration. Some of your changes may conflict with some of those.

Hello Tim,
it wasn't finished
I had to fix the missing word format in the process because it was broken (always the same thing: the answer_format and feedback_format fields). It now seems to work both for shortanswer and multichoice. I have added the files I used to test it in question/format/missingword/tests/fixtures
I am not really sure it is used by anybody because I found no report that it was broken !!
I am unsure if I must delete the $string['unknownorunhandledtype'] from blackboard_six or not now that I have created exactly the same lang string in question component. I seems to remember it is forbidden to delete strings on stable branchs ?

Yes I just noticed the MDL-37068 dependence! How to deal with that ?

Correct, you cannot delete strings on stable branches, so leave the unused string there, but delete it from master. (Really, you should write an AMOS script http://docs.moodle.org/dev/Languages/AMOS#AMOS_script so that the string automatically gets moved in all language packs.)

About MDL-37068, I think we may have to wait until that is integrated, then you can re-base your patch on top of it.

Hello Tim,
Now that MDL-37068 is closed, I need to finish this one.

• Squash all commits.
• Do branchs for Moodle 2.3 and 2.4 without the string removal
• For the AMOS script I think I understand what I should put in it but it's unclear where I should put this script after that.
• Write testing instructions, I suppose we need to test Missing word format as it was fixed in the process but for the hardcoded strings, how to test ?

I put it in peer review so that you can have a look at the code before I do the final changes in the above list

Looking good.

The AMOS script should go in the commit comment.

The main thing we need to test is that nothing is broken, so import some example files of each type.

For missingword, there are two examples in the comment at the top of question/format/missingword/format.php. It would be good to save those into tests/fixtures as an example file.

Those 2 examples (and a third one) are already part of the branch . I have put them when I discovered them in the code and used them for testing.

Doh! sorry. There were such short files I missed them.

Just updating workflow state. I think I am waiting for you to do the things listed above.

Ping! Where have we got to with this?

Hello Tim,
Sorry but new semester has started so I can only work on Moodle during the week-ends ... I hope to be able to finish this one tomorrow and Sunday.

That is OK. There is no need to apologise. I just wanted to make sure that you were not waiting for me to do anything. I am away for most of this weekend, but will probably check to see if there is anything for me to peer review once I get home on Sunday evening.

Hello Tim,
rebasing and squashing done.
If I am right, master branch should have an AMOS script:
AMOS BEGIN
MOV [unknownorunhandledtype,qformat_blackboard_six],[unknownorunhandledtype,question]
AMOS END
in the commit comment.
But what about the stable branchs ?
As I don't delete the string and create a new one should I include an AMOS script to copy the string ?
AMOS BEGIN
CPY [unknownorunhandledtype,qformat_blackboard_six],[unknownorunhandledtype,question]
AMOS END
That seems logical to me, but not really sure as it's my first time with AMOS scripts.

That looks right to me.

David Mudrak, if you see this, please could you confirm.

Thanks. Submitting for integration.

Thanks I have added some testing instructions. I hope my AMOS scripts are OK. But I think integrators will verify them.
Have a nice week-end Tim.

Yes, the AMOS script looks ok.

Thanks David.

Thanks guys, this has been integrated now.

Tested in master. Looks good.

Also tested in 2.3 and 2.4. Passing.

A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed.

(and won't be revisiting it unless some regression is found)

Thanks and ciao