4.6 - upgrade error with Shared Question Banks

[aspark21]

Testing Moodle 4.6 for https://tracker.moodle.org/browse/MDLQA-19535

The following failures occur with this plugin that do not occur with 4.5

-->qtype_coderunner
Default exception handler: error/Invalid contextlevel for target category, must be CONTEXT_MODULE Debug: 
Error code: Invalid contextlevel for target category, must be CONTEXT_MODULE
$a contents: 
* line 146 of /question/format.php: core\exception\moodle_exception thrown
* line 182 of /question/type/coderunner/db/upgradelib.php: call to qformat_default->setCategory()
* line 166 of /question/type/coderunner/db/upgradelib.php: call to load_questions()
* line 50 of /question/type/coderunner/db/upgradelib.php: call to load_new_prototypes()
* line 437 of /question/type/coderunner/db/upgrade.php: call to update_question_types()
* line 7[57](https://github.com/ucl-isd/moodle-mysql-restore/actions/runs/12131616522/job/33824116798#step:8:58) of /lib/upgradelib.php: call to xmldb_qtype_coderunner_upgrade()
* line 1936 of /lib/upgradelib.php: call to upgrade_plugins()
* line 274 of /admin/cli/upgrade.php: call to upgrade_noncore()

!!! error/Invalid contextlevel for target category, must be CONTEXT_MODULE !!!
!! 
Error code: Invalid contextlevel for target category, must be CONTEXT_MODULE
$a contents:  !!
!! Stack trace: * line 146 of /question/format.php: core\exception\moodle_exception thrown
* line 182 of /question/type/coderunner/db/upgradelib.php: call to qformat_default->setCategory()
* line 166 of /question/type/coderunner/db/upgradelib.php: call to load_questions()
* line 50 of /question/type/coderunner/db/upgradelib.php: call to load_new_prototypes()
* line 437 of /question/type/coderunner/db/upgrade.php: call to update_question_types()
* line 757 of /lib/upgradelib.php: call to xmldb_qtype_coderunner_upgrade()
* line 1936 of /lib/upgradelib.php: call to upgrade_plugins()
* line 274 of /admin/cli/upgrade.php: call to upgrade_noncore()

[timhunt]

Yes. Moodle 5.0 / 4.6 will change how the question bank works, which means that the age-old hack (is that fair?) in CodeRunner that prototypes are stored in the system question bank will not work any more. There is no longer a system question bank.

So, this model needs a re-think. I have some thoughts about how best to deal with that, if you want to discuss it @trampgeek.

[timhunt]

No need for me to be mysterious. My outline solution is:

1. There should be a specific Question bank (mod_qbank instance) in the front-page course, with a name like 'CodeRunner prototypes. Therefore, CodeRunner, install/upgrade should look for that, and auto-create if it does not already exist. (I would identify it using a suitable course_modules.idnumber, like qtype_coderunner-system-prototypes. (Here is the code to create a mod_qbank)
2. To link questions to their prototype, we should stop using name-matching, and searching through contexts. Instead, we should make the link explicit, using the question_references DB table.
   • component would be 'qtype_coderunner'.
   • area could be 'prototype'.
   • usingcontextid is the context where the question using the prototype is - will need a question_moved event handler to keep this up to date). itemid is the question id of the question using the prototype.
   • questionbankentryid identifies which prototype this question uses.
   • version - I expect people will mostly set this to 'Always latest' (null), but acutally, there might be uses for being able to specify a fixed version of the prototype.
3. Then, when editing a CodeRunner question, to select the prototype, there should be a UI like when you pick a question to add to a quiz, to pick a prototype question.

An advantage of using question_references is that then, for example, the question bank will prevent people from deleting prototyes that are in use.

[trampgeek]

Ouch. This is very worrying. We're only just recovering from the introduction of question versioning :-)

We're worried about the idea of linking questions to their prototypes using the question references DB table. Some of the concerns have already been outlined on a CodeRunner forum posting: https://coderunner.org.nz/mod/forum/discuss.php?d=570 There, we talk about the problems with exporting a quiz but similar and possibly even more serious concerns arise when backing up and restoring courses.

For example, suppose we have a question bank of prototypes that we share between two courses. We now backup both courses at the end of the academic year, ready for setting up our new year's server. How do we manage the question links in this case. Do both course backups include the shared question bank? Or is that backed up separately? In both cases, there seems to be a problem in re-establishing the question references.

[trampgeek]

I've started to look into this, Tim. But when I call the required question_bank_helper functions I get an exception thrown to say that they can't be used during an upgrade. I've worked around that with the following bit of code, but I feel very uncomfortable with that approach. There is presumably a good reason not to use those functions during upgrades so switching off the checks seems dangerous. Can you reassure me it's safe please, or alternatively suggest a better approach?

$bankname = get_string('systembank', 'question');
try {
	// WARNING - dangerous code ahead ...
	$savedupgradestatus = $CFG->upgraderunning;  // Should be true, but let's play it safe.
	$CFG->upgraderunning = false;
	if (!$newmod = question_bank_helper::get_default_open_instance_system_type($course)) {
		$newmod = question_bank_helper::create_default_open_instance($course, $bankname, question_bank_helper::TYPE_SYSTEM);
	}
	$CFG->upgraderunning = $savedupgradestatus;
}  catch (Exception $e) {
	$CFG->upgraderunning = $savedupgradestatus;
	throw new coding_exception('Upgrade failed: error creating system question bank');
}

EDIT My concerns look justified. While the above code worked when I did a manual Moodle install (adding CodeRunner after getting the basic Moodle working) it failed in behat testing. Specifically the line

$module = $DB->get_record('modules', ['name' => self::get_default_question_bank_activity_name()], '*', MUST_EXIST);

throws a "record not found" exception. Implying to me that CodeRunner installer is being run before the qbank module has been installed.

[timhunt]

Your observation about Behat install is correct. The order that Moodle plugins are installed puts question types before activity modules. But, best to avoid relying on the order if you can.

I think the best way to deal with this - and the main problem you asked about - is to use an ad-hoc task.

1. In install (and upgrade if desired) queue an instance of your newly created \qtype_coderunner\task\update_system_prototypes task
2. That task will then run immediatly after install (and as part of Behat or PHPunit install), and can use methods like get_default_open_instance_system_type.

My instinct would be to code this in a way that is idempotnent, so it look for a qbank with idnumber (in the course_modules table) set to something appropriate like 'qtype_coderunner-system-prototypes' and only creates it if it does not already exist. This task can then proceed to do what the existing update_question_types function does.

Also, if you have not already looked at mod/qbank/db/install.php I mean mod/qbank/classes/task/transfer_question_categories.php then that might give some useful ideas.

I hope that is helpful.

[trampgeek]

Many thanks Tim. Very helpful. I had suspected queuing a task would be the way to but it's good to have it confirmed. I had wondered how to notify any errors that occurred, given the task is now asynchronous with the install/upgrade, but no doubt I can find a way to at least log an error.

Yes, I had looked at the transfer_question_categories.php file, as you suggested earlier. It's how I was able to get the install code written (which does work if you install CodeRunner after installing Moodle) - many thanks for the earlier link to it. Wouldn't have had a clue, without that.

I'll give that a go tomorrow.

[timhunt]

Output from talks (mtrace) get captured, and there is an admin UI where people can chech that.

Also, if you task fails (ends by throwing an exception rather than completing) the system tracks that and retries etc.

Certainly, at the OU we monitor what all the tasks are doing (using report_customsql to email admins every day if any tasks have failed).

[trampgeek]

I'm having some problems with implementing this. I wrote an adhoc task that set up the Front Page question bank, added a category for CodeRunner and uploaded all the question prototypes. But it just didn't seem to be working. After some confusing debugging I discovered that it was in fact working fine but the transfer_question_categories task, which ran after it, was then deleting my question category and all its questions!

It seems that the functions question_category_in_use and question_category_delete_safe operate on the assumption that questions that aren't actually used explicitly in quizzes are junk and can be deleted. This is certainly a seriously flawed assumption for CodeRunner prototypes but also seems dangerous for question bank development in general. We often have question categories containing questions we're developing for tests and exams, or have removed from current quizzes but want to keep for possible future use.

I thought I might be able to deal with the immediate problem by failing the adhoc task if the front page question bank wasn't present yet, so that it could be retried later. But it seems that retrying of failed adhoc questions doesn't take place during install/upgrade?

One further issue is that logging of adhoc tasks, including their mtrace output, doesn't seem to take place during install/upgrades either? So notifying admins of errors seems problematic. However, I would hope that errors shouldn't be occurring once the fundamental issues are dealt with.

[timhunt]

The problem with the questoins being deleted might be https://tracker.moodle.org/browse/MDL-83977. Are you testing aganist the latest main branch of Moodle? There was a flawed assumption, but it should be fixed now.

[trampgeek]

Ah, thanks Tim. I was indeed testing against an older branch. Good to know the issue has been addressed.