question_type::initialise_question_instance should use extra_question_fields to get data from $questiondata->options

Tim, I'm writing this patch and stumble once more in the cursed redundant "answers" field in the shortanswer question table. You don't fill it in question object, yet it remains to be filled by questiontype class. That prevents writing solution to this issue.

Could I remove handling of "answers" fields from the question type (i.e. save_question_options/get_question_options) too in this git commit (it may be leaved to be in DB)? Please? My testing site still works without this field at all for more than two years now (with only problems on upgrade to 2.1 where it's hardcoded), I have written you a patch to remove it long ago, and this nuisance still persists...

If you intend to keep it in question type class, then it should be added to the question class too to allow extra_question_fields to work universally.

P.S. I understand that you deleted it from the new class while preserving in the old class. But given the scale of the issue it's hardly necessary, and it's really gives problems... Could we solve it at last?

Tim, please apply it fast, since conversion of preg for 2.1 depend on this. As always, shortanswer is converted and tested.

I written a solution for this and even commit it to my git repository, but have trouble pushing it to github due to infamous "git under non-english version of windows" problem.

So, because it is urgent, I generated a patch with git. The patch is done against Moodle 2.1 stable, but I don't think you'll have problem applying it to the master branch too.

If you want your patch applied fast, it has to be 100% right.

1. If you want to get rid of the answers field, you need to do it properly, and remove it everywhere. (All affected qtypes, DB fields, backup/restore code, etc.) We already have a separate issue for that, don't we? Yes MDL-17812 - So I don't like you change in public function extra_question_fields() {, and I don't see why it is necessary alongside the rest of your changes.

2. The first line of the commit comment is way too long.

3. Is extra_question_fields used everywhere else that is necessary? e.g. backup and restore?

4. Please can you add some testing instructions to this bug.

1 & 2 Ok, I remove line wich deletes 'answers' out of extra_question_fields and shorten first line (any rules on it's length?) and reupload patch shortly

3. The issue with backup and restore & extra_question_fields is MDL-25617. It is stalled because first you waited to review it so long I lost access to the people I used to get it done, and when you reviewed it at last I was already missing both time and helpers to fix problems you found (mostly trivia). I remember about it but far too busy to do something (you know my timezone is Moscow, so just look at the time of my attachment...) right now.

Meanwhile, it is broken since Moodle 2.0 release and I don't get any single complaint about it, while I already got several ones about not releasing preg for Moodle 2.1. (and I still have to deal with behavours and penalty calculation). So I consider this issue, which stop all question from working, far more urgent than backup/restore thing.

4. Testing with core qtype ensures that handling of usecase field in shortanswer working correctly:
1) create shortanswer question with usecase on
2) re-edit it to ensure usecase value is showed correctly on the edit form
3) try question in the preview, changing case of the correct answer, so you are sure case is taken into account
4) you may repeat these steps with usecase off (I did), thought it less necessary since this is default mode

'answers' field remained and first line of commit shortened.

Thought that was much easy to do without using git, reverting commits etc...

Thanks Oleg. Submitting for integration now.

Note that this change will not break any third-party question types (although it does make them slightly easier to write.)

(Just one more thing. We tend not to used Signed-off by in Moodle, for some reason. Better if you don't use it with future patches, but it does not matter here.)

Tim, will these set of fix branches mean preg isn't going to work for Moodle 2.1 at all, only for 2.2?

Well, I was being risk-averse. Looking again, it is safe to do this in 2.1 too. New branch made.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

PS: Note this is the last message of this type that you will receive along the whole November month, because we are running continuous integration this weeks while QA tests for release of Moodle 2.2 (Dec 1st) are being performed.

Hi All,

We (NetSpot) have a couple of clients that use this qtype in 2.0.x looking to upgrade to 2.1/2.2 and not being able to have this qtype is seen as a bit of a blocker to them.

+1 to getting this in

Integration of this patch (which unbreaks once more "extra_question_fields" feature) into Moodle code hinders release of Preg question type for Moodle 2.1. I have quite a number of requests for getting Preg working for 2.1, so the faster we get this in - the better.

P.S. Eloy, it's useless to ask me for rebase patch since (as you could see in the path) Tim is maintaining these branches on the server. Still I have no trouble rebasing these on local repository.

Hi,
I'm a heavy user of regular expressions for my quizes, PREG is ideal for me because allows the use of open regexps. I'm prof atthe School of Medicine of the University of Puebla in Mexico, these questions allow to write a diagnosis with a reasonable freedom of redaction and without suggesting answers as in multiple choice questions. I would be great to have it in the core course, not as a plug in.
Thanks
Luis

Integrated, thanks!

I was a bit annoyed because, in the patch I only saw how the old "usecase" code was being deleted but the realized that qtype_shortanswer->extra_question_fields() already has it.

PS1: Plz, try to put one space between the '//' and the comments. Not blocker but a coding recommendation.

PS2: The message above about suggesting to rebase is sent automatically to all issues pending to integrate after git repos have been updated, aiming to reduce conflict chances (only happening rarely, mostly when there is some upgrade script involved, or certain area of code is being modified by various developers independently).

Test passed. Answers are still case sensitive if usecase is on.

And this has landed upstream, just on time for the upcoming new releases week. Thanks for it!

Ciao