From b5fd7946f0d0ea857c2af551b2cf0609f25ad238 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Thu, 26 Feb 2026 09:28:51 +0000 Subject: [PATCH] MDL-86691 questions: Fix incorrect set reference category contexts During the upgrade to 5.x, question categories are moved to new mod_qbank module contexts, and any set references using those categories are updated using `move_question_set_references()`. However, this function wasn't updating the `cat` arribute of the set reference's filter condition, leaving it pointing at the original context ID. This change update `move_question_set_references()` so that when a set reference is moved (during the 5.x upgrade, or otherwise) the `cat` parameter will be updated correctly. For sites that have already gone through the 5.x upgrade, this fix also adds a CLI script to find and fix incorrect question_set_reference records. I considered adding this as an upgrade step, but it potentially has a lot of records to check, and it's redundant if the site is being upgraded to 5.x with this fix already in place. --- lang/en/question.php | 2 + lib/questionlib.php | 3 +- .../classes/question_reference_manager.php | 29 +++++++ .../fix_set_references_category_context.php | 64 +++++++++++++++ question/editlib.php | 7 +- .../tests/question_reference_manager_test.php | 78 +++++++++++++++++++ 6 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 question/cli/fix_set_references_category_context.php diff --git a/lang/en/question.php b/lang/en/question.php index e2cda045133..6924486fe19 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -211,6 +211,8 @@ $string['importquestions_link'] = 'question/import'; $string['importwrongfileencoding'] = 'The file you selected does not use UTF-8 character encoding. {$a} files must use UTF-8.'; $string['importwrongfiletype'] = 'The type of the file you selected ({$a->actualtype}) does not match the type expected by this import format ({$a->expectedtype}).'; $string['invalidarg'] = 'No valid arguments supplied or incorrect server configuration'; +$string['invalidcategory'] = 'Invalid category provided for this question bank view.'; +$string['invalidcategoryeditq'] = 'Invalid category provided for this question bank view. Ask your administrator to try running the question/cli/fix_set_references_category_context.php script.'; $string['invalidcategoryidforparent'] = 'Invalid category id for parent!'; $string['invalidcategoryidtomove'] = 'Invalid category id to move!'; $string['invalidconfirm'] = 'Confirmation string was incorrect'; diff --git a/lib/questionlib.php b/lib/questionlib.php index 8dcc192844e..a6d7a38db30 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -687,8 +687,9 @@ function move_question_set_references(int $oldcategoryid, int $newcatgoryid, && $oldcategoryid !== $newcatgoryid ) { $filter['filter']['category']['values'][0] = $newcatgoryid; - $setreference->filtercondition = json_encode($filter); } + $filter['cat'] = implode(',', [$filter['filter']['category']['values'][0], $newcontextid]); + $setreference->filtercondition = json_encode($filter); $DB->update_record('question_set_references', $setreference); } $setreferences->close(); diff --git a/question/classes/question_reference_manager.php b/question/classes/question_reference_manager.php index 5c1a4e4974c..91821df5f45 100644 --- a/question/classes/question_reference_manager.php +++ b/question/classes/question_reference_manager.php @@ -136,4 +136,33 @@ class question_reference_manager { } return $filtercondition; } + + /** + * Ensure consistency of filter 'cat' parameter and questioncontextid in all set references. + * + * Some set references may have been moved to a different context, but the filter condition not updated with the context ID. + * Since the filter condition is JSON-encoded, we have to check each set reference record for inconsistencies. + * + * This is used in a CLI script to fix bad data due to MDL-86691. + * + * @return int The number of records that were updated. + * @todo Deprecate in Moodle 6.0 (MDL-87844) for removal in 7.0 (MDL-87845). + */ + public static function fix_set_references_category_context(): int { + global $DB; + $updates = 0; + $sets = $DB->get_recordset('question_set_references'); + foreach ($sets as $set) { + $filtercondition = json_decode($set->filtercondition, true); + [$catid, $catcontext] = explode(',', $filtercondition['cat']); + if ($catcontext != $set->questionscontextid) { + $filtercondition['cat'] = implode(',', [$catid, $set->questionscontextid]); + $set->filtercondition = json_encode($filtercondition); + $DB->update_record('question_set_references', $set); + $updates++; + } + } + $sets->close(); + return $updates; + } } diff --git a/question/cli/fix_set_references_category_context.php b/question/cli/fix_set_references_category_context.php new file mode 100644 index 00000000000..737855e3107 --- /dev/null +++ b/question/cli/fix_set_references_category_context.php @@ -0,0 +1,64 @@ +. + +/** + * Fix bad set reference data due to MDL-86691. + * + * @todo Deprecate in Moodle 6.0 (MDL-87844) for removal in 7.0 (MDL-87845). + * + * @package core_question + * @copyright 2026 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +define('CLI_SCRIPT', true); + +require(__DIR__ . '/../../config.php'); +require_once($CFG->libdir . '/clilib.php'); + +[$options, $unrecognized] = cli_get_params(['help' => false], ['h' => 'help']); + +if ($unrecognized) { + $unrecognized = implode("\n ", $unrecognized); + cli_error(get_string('cliunknowoption', 'admin', $unrecognized), 2); +} + +if ($options['help']) { + $help = <<count_records_select("question_categories", "id = ? AND contextid = ?", array($catparts[0], $catparts[1]))) { - throw new \moodle_exception('invalidcategory', 'question'); + $exception = 'invalidcategory'; + if ($edittab === 'editq') { + // If this might be due to MDL-86691, return a message including fix instructions. + $exception = 'invalidcategoryeditq'; + } + throw new \moodle_exception($exception, 'question'); } } else { $category = $defaultcategory; diff --git a/question/engine/tests/question_reference_manager_test.php b/question/engine/tests/question_reference_manager_test.php index cba3910abab..e4040175cec 100644 --- a/question/engine/tests/question_reference_manager_test.php +++ b/question/engine/tests/question_reference_manager_test.php @@ -120,4 +120,82 @@ final class question_reference_manager_test extends advanced_testcase { question_reference_manager::questions_with_references([-1])); } + + /** + * Any question set references where questioncategoryid does not match filtercondition['cat'] should have the cat updated. + * + * @todo Deprecate in Moodle 6.0 (MDL-87844) for removal in 7.0 (MDL-87845). + */ + public function test_fix_set_references_category_context(): void { + global $DB; + $this->resetAfterTest(); + + $correctreference = (object) [ + 'usingcontextid' => 1, + 'component' => 'core_question', + 'questionarea' => 'test', + 'itemid' => 1, + 'questionscontextid' => 2, + 'filtercondition' => json_encode( + [ + 'filter' => [ + 'category' => [ + 'name' => 'category', + 'jointype' => 1, + 'values' => [1], + 'filteroptions' => [ + 'includesubcategories' => 0, + ], + ], + ], + 'cmid' => 1, + 'courseid' => 1, + 'cat' => '1,2', + ], + ), + ]; + $correctreference->id = $DB->insert_record('question_set_references', $correctreference); + + $incorrectreference = (object) [ + 'usingcontextid' => 1, + 'component' => 'core_question', + 'questionarea' => 'test', + 'itemid' => 2, + 'questionscontextid' => 4, + 'filtercondition' => json_encode( + [ + 'filter' => [ + 'category' => [ + 'name' => 'category', + 'jointype' => 1, + 'values' => [6], + 'filteroptions' => [ + 'includesubcategories' => 0, + ], + ], + ], + 'cmid' => 1, + 'courseid' => 1, + 'cat' => '6,3', + ], + ), + ]; + $incorrectreference->id = $DB->insert_record('question_set_references', $incorrectreference); + + $fixedcount = question_reference_manager::fix_set_references_category_context(); + + $this->assertEquals(1, $fixedcount); + + $updatedcorrectrefrence = $DB->get_record('question_set_references', ['id' => $correctreference->id]); + $this->assertEquals($correctreference, $updatedcorrectrefrence); + + $updatedincorrectrefrence = $DB->get_record('question_set_references', ['id' => $incorrectreference->id]); + $this->assertEquals($incorrectreference->usingcontextid, $updatedincorrectrefrence->usingcontextid); + $this->assertEquals($incorrectreference->questionscontextid, $updatedincorrectrefrence->questionscontextid); + $filtercondition = json_decode($updatedincorrectrefrence->filtercondition, true); + $this->assertEquals( + implode(',', [$filtercondition['filter']['category']['values'][0], $updatedincorrectrefrence->questionscontextid]), + $filtercondition['cat'], + ); + } }