Moodle

Hi, Tim!

I see, this issue is not on you priority list (thought it takes several minutes to fix).
We still badly need such questions right now.
If I make a patch., will you apply it in Moodle codebase for 1.9 and 2.0?

Well, like most things, it would take some minutes to fix, half an hour to test, and more time to merge into 1.9 and 2.0 - and there are plenty of other things on my todo list.

However, it would be really great if you could make a patch, and I will certainly review it, and either apply it, or tell you what is wrong with it Thanks.

Validate function changed to allow questions with 2 subquestions and 3 subanswers

Corresponding lang string file.

Corresponding lang string file (need to be edited for filloutthreequestions string).

I created a patch and tested it not only on our testing site, but on our production site as well. All seems to work OK.

This patch will hardly need any thorought reveiwing as its affects only one local function (validate), and two language strings. There are no real programming need to these restrictions, this is a logical restriction only,.

P.S. Probably I must introduce myself.
Sychev O.A., Docent of the Software development department of the Volgograd State Technical University (Russia).

We are running pilot project on using Moodle in our University as leading CMS,

If you approve this, I can make a version for 2.0 myself, if there is any difference in validating questions between 1.9 and 2.0

Looks like you have been doing great work while I have been on holiday. Please could you attach the patch file here, that would be much easier for me to review and check in. Thanks. (http://docs.moodle.org/en/Patch, if you need it.)

Patches were created using WinMerge in Unified format. If something is wrong, please tell me - this is my first patches.

Actually, there is only one potential problem with this patch: updating language packs to meet new interface for notenoughquestions string in qtype_match.php file. It already recieved one number, so I see no point to hardcode another number in the string and changed parameter instead. If there are better solution please tell me.

After scanning all 1.9.2 codebase I didn't find any other use of the updated string, so it is safe to change it (thought it's location in quiz.phph instead of qtype_match.php puzzles me).

The lang string location is historical. Back in the day of Moodle 1.4 or 1.5, all the questions were part of the quiz module. When they were separated the people doing the work did not bother to move all the language strings. They are only gradually being moved.

However, the rules are that you must not change language strings. This is for two reasons. 1. All versions of Moodle use the same language packs, so you should only add strings for that reason; and also, the translators tools do not tell them when strings are changed, only when there are strings in the english files that are not in their files. So new strings have to be new to get translated.

So

Anyway, thank you for creating the patches. My comments:

1. As the Coding guidelines (http://docs.moodle.org/en/Development:Coding) say, you must indent your code with 4 spaces, not tabs.

2. As above, create a new string notenoughqsandas instead of changing notenoughquestions. Similarly, don't change filloutthreequestions, instead make a new string filloutthreequestionsandtwoanswers in qtype_match.php.

3. I am not sure about your change to the line
if ($trimmedanswer != '' && $trimmedquestion != ''){
I suppose it does not really matter because of the test below
if ($trimmedquestion != '' && $trimmedanswer == ''){
The only thing is, which gives the less confusing display when someone enters questions without answers?

4. We could clean up the code that sets the errors array to reduce the duplication. Something like:
if ($questioncount < 1){ $errors['subquestions[0]'] = get_string('notenoughquestions', 'qtype_match', $numberqanda); }
if ($questioncount < 2){ $errors['subquestions[1]'] = get_string('notenoughquestions', 'qtype_match', $numberqanda); }
if ($answercount < 3){ $errors['subanswer[2]'] = get_string('notenoughquestions', 'qtype_match', $numberqanda); }

Anyway, this is nearly good enough to be committed, so it would be great if you could just make the changes I suggest. Thanks.

Thanks for kindly description of language strings problem.

1. Find one string with tab, fixed.
2. Fixed. I can also add new string to qtype_match.php instead of quiz.php, if you want it (this will also reduce the quantity of patches).
3. I'm unsure too. I see no point in keeping if ($trimmedanswer != '' && $trimmedquestion != ''){ because if user will enter third question without answer, the display will be the same - notenoughtquestions will replace previous warning; if user additionaly enters first or second questions without answers he/she will see two errors per pair in old system, and only one in my version. But if you insist, I can keep this string as it was.
4 Fixed.

I wait for you conclusion on moving the string and 3. before submitting final patches.

2. Yes, please move the new string.

3. I am thinking of the following situation:

Q1 [United Kingdom] A1 [London]
Q2 [Australia ] A2 [ ]
Q3 [ ] A3 [Paris ]

In this case, I think it would be best to just display one error next to A2, saying that that you are not allowed a blank answer next to a non-blank question. I think it would be better not to display an error next to Paris in this case. Your code would do that.

Ah, so now I think the correct logic is:
if ($trimmedquestion != ''){ $questioncount++; }

if ($trimmedanswer != '' || $trimmedquestion != ''){ $answerscount++; }

2. Fixed.
3. Fixed. I tested results on our site - all works as expected.
Patches were created based on Moodle 1.9. Please let me know if any 2.0 adaptation is necessary.

Just FYI, this is the kind of patch that CVS generates. Note that it puts the changes to all files into one patch, which is more convenient.

Fix now checked in to 1.9 and HEAD. Thanks Oleg for your work implementing this.

I tested Oleg's patch before committing it, so I guess it is OK for me to close this issue as well as resolving it.

Tim, many thanks for working with me on this issue and kindly remarks!

If you would be so kind to continue our work, there is two another issues with quiz and questions in Moodle that pesters our staff and needs to be removed.

One is rather major (comapring to this one), but also very annoing issue with usability of question editing form - we have thousands of questions, and a lot of teachers with different requests - under some circumstances creating/editing questions is very unusable (while under other it just somewhat annoying). There are some things that can greatly increase usablility of this page under any conditions, retaining old behavour whenever possible. I will discuss proposals with you and after you approval create a patch and test it on our university site. As somewhat major change this probably must go to 2.0 only.

The other is rather minor one, thought it puzzles people. Currently review system will show students all questions even if they have right to review only grade for this quiz. They can see the grade anyway, so it's not that important, but it sometimes results in information leakage from unexperienced tachers, who supposes that students will see only grade, not questions. This patch probably can be done just when I have free hour or two.

If you would be so kind to review and apply these patches (especially first), I will create separate issues for them and continue to work there.

I would be very happy to continue working with you. One of the reasons I took the time to help you with this fix was in the hope that it would lead to more fixes in future and the way Open Source progresses is by everyone fixing the things that annoy them most, until everything is perfect.

I totally agree about the question editing forms needed their usability improved. Instead of just discussing the issue with me, it would be better to discuss it in the quiz forum http://moodle.org/mod/forum/view.php?f=121 because it is not so important what I think about your changes. It is much more important what other people who use the quiz think about your ideas. However, once the discussion has happened, a patch attached to a separate tracker issue is what is needed.

I am not so sure about the review options patch. I would need to see a more detailed summary of how it works now, and how you propose to change it. Again, the place for that sort of discussion is the quiz forum. Thanks.