diff --git a/.upgradenotes/MDL-86524-2025120211115999.yml b/.upgradenotes/MDL-86524-2025120211115999.yml new file mode 100644 index 00000000000..5a72101c37f --- /dev/null +++ b/.upgradenotes/MDL-86524-2025120211115999.yml @@ -0,0 +1,12 @@ +issueNumber: MDL-86524 +notes: + core_question: + - message: >- + During restore of a question_set_reference, mapping of IDs in the + filtercondition is now delegated to qbank plugins. + If your qbank plugin defines a filter condition that uses database + IDs, add an override of `restore_filtercondition()` to the `condition` + class, which checks the condition's data and replaces the IDs with + mapped values if required. See + `qbank_managecategories\category_condition` for an example. + type: improved diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index daa0cb6cad7..86994a1576e 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -5472,12 +5472,18 @@ class restore_create_categories_and_questions extends restore_structure_step { } $tagcontextid = $this->cachedcategory->contextid; // Add the tag to the question. - core_tag_tag::add_item_tag('core_question', + $taginstanceid = core_tag_tag::add_item_tag( + 'core_question', 'question', $newquestion, context::instance_by_id($tagcontextid), - $tagname + $tagname, ); + $tagid = $DB->get_field('tag_instance', 'tagid', ['id' => $taginstanceid]); + if ($tagid != $data->id) { + // The tag didn't exist already, map the new ID. + $this->set_mapping('tag', $data->id, $tagid); + } } } @@ -5682,13 +5688,19 @@ class restore_move_module_questions_categories extends restore_execution_step { // We need to check all the question_set_references belonging to this context_module. $references = $DB->get_records('question_set_references', ['usingcontextid' => $newcontext->newitemid]); foreach ($references as $reference) { - $filtercondition = json_decode($reference->filtercondition); - if (!empty($filtercondition->questioncategoryid) && - in_array($filtercondition->questioncategoryid, $categoryids)) { - // This is one of ours, update the questionscontextid. - $DB->set_field('question_set_references', - 'questionscontextid', $newcontext->newitemid, - ['id' => $reference->id]); + $filtercondition = json_decode($reference->filtercondition, true); + if (!array_key_exists('filter', $filtercondition)) { + $filtercondition = \core_question\question_reference_manager::convert_legacy_set_reference_filter_condition( + $filtercondition, + ); + } + $questioncategoryid = $filtercondition['filter']['category']['values'][0]; + if (in_array($questioncategoryid, $categoryids)) { + // This is one of ours, update the questionscontextid and filtercondition fields. + $reference->questionscontextid = $newcontext->newitemid; + $filtercondition['cat'] = "{$questioncategoryid},{$newcontext->newitemid}"; + $reference->filtercondition = json_encode($filtercondition); + $DB->update_record('question_set_references', $reference); } } } @@ -6495,50 +6507,41 @@ trait restore_question_set_reference_data_trait { public function process_question_set_reference($data) { global $DB; $data = (object) $data; - $owncontext = $data->usingcontextid == $data->questionscontextid; $data->usingcontextid = $this->get_mappingid('context', $data->usingcontextid); $data->itemid = $this->get_new_parentid('quiz_question_instance'); + + if ($context = $this->get_mappingid('context', $data->questionscontextid)) { + $data->questionscontextid = $context; + } else { + $this->log( + "question_set_reference with old id {$data->id} referenced question context " + . "{$data->questionscontextid} which was not included in the backup. Therefore, this has been " + . "restored with the old questionscontextid.", + backup::LOG_WARNING, + ); + } + $filtercondition = json_decode($data->filtercondition, true); if (!isset($filtercondition['filter'])) { // Pre-4.3, convert the old filtercondition format to the new format. + // Don't map tags to new IDs, the plugin will do that below. $filtercondition = \core_question\question_reference_manager::convert_legacy_set_reference_filter_condition( - $filtercondition); + $filtercondition, + false, + ); } - // Map category id used for category filter condition and corresponding context id. - $oldcategoryid = $filtercondition['filter']['category']['values'][0]; - // Decide if we're going to refer back to the original category, or to the new category. - // Are we restoring to a different site? - // Has the original context or category been deleted? - // Did the old category belong to the same context as the original set reference? - // Are we allowed to use its questions? - $questionscontext = context::instance_by_id($data->questionscontextid, IGNORE_MISSING); - if ( - !$this->get_task()->is_samesite() - || !$questionscontext - || !$DB->record_exists('question_categories', ['id' => $oldcategoryid]) - || $owncontext - || !has_capability('moodle/question:useall', $questionscontext) - ) { - $newcategoryid = $this->get_mappingid('question_category', $oldcategoryid); - $filtercondition['filter']['category']['values'][0] = $newcategoryid; + $qbankfeatureclasses = \core\component::get_plugin_list_with_class('qbank', 'plugin_feature'); - if ($context = $this->get_mappingid('context', $data->questionscontextid)) { - $data->questionscontextid = $context; - } else { - $this->log('question_set_reference with old id ' . $data->id . - ' referenced question context ' . $data->questionscontextid . - ' which was not included in the backup. Therefore, this has been ' . - ' restored with the old questionscontextid.', backup::LOG_WARNING); + foreach ($qbankfeatureclasses as $qbankfeatureclass) { + $qbankfeature = new $qbankfeatureclass(); + $filters = $qbankfeature->get_question_filters(); + foreach ($filters as $filter) { + $filtercondition = $filter->restore_filtercondition($filtercondition, $data, $this); } } - $filtercondition['cat'] = implode(',', [ - $filtercondition['filter']['category']['values'][0], - $data->questionscontextid, - ]); - $data->filtercondition = json_encode($filtercondition); $DB->insert_record('question_set_references', $data); diff --git a/mod/quiz/tests/backup/restore_39_test.php b/mod/quiz/tests/backup/restore_39_test.php index 4a0c4e0746b..226c0a8ae7d 100644 --- a/mod/quiz/tests/backup/restore_39_test.php +++ b/mod/quiz/tests/backup/restore_39_test.php @@ -74,10 +74,8 @@ final class restore_39_test extends advanced_testcase { // Get question_set_references records for the restored quiz activity. $references = $DB->get_records('question_set_references', ['usingcontextid' => $contextid]); foreach ($references as $reference) { - $filtercondition = json_decode($reference->filtercondition); - // Confirm the questionscontextid is set correctly, which is from filter question category id. - $this->assertEquals($reference->questionscontextid, - $qcats[$filtercondition->questioncategoryid]->contextid); + // Confirm the questionscontextid is set correctly, which is now the quiz context. + $this->assertEquals($contextid, $reference->questionscontextid); } } } diff --git a/mod/quiz/tests/quiz_question_restore_test.php b/mod/quiz/tests/quiz_question_restore_test.php index 629bdf9ce10..f9063def584 100644 --- a/mod/quiz/tests/quiz_question_restore_test.php +++ b/mod/quiz/tests/quiz_question_restore_test.php @@ -616,11 +616,7 @@ final class quiz_question_restore_test extends \advanced_testcase { $setreference = $DB->get_record('question_set_references', ['itemid' => $slot->id, 'component' => 'mod_quiz', 'questionarea' => 'slot']); $filterconditions = json_decode($setreference->filtercondition); - $tags = []; - foreach ($filterconditions->tags as $tagstring) { - $tag = explode(',', $tagstring); - $tags[] = $tag[1]; - } + $tags = array_map(fn($tag) => $tag->name, \core_tag_tag::get_bulk($filterconditions->filter->qtagids->values)); $this->assertEquals([], array_diff($randomtags[$slot->slot], $tags)); } @@ -765,6 +761,11 @@ final class quiz_question_restore_test extends \advanced_testcase { /** * Test pre 4.3 quiz restore for random question filter conditions. * + * This performs a high-level check that the filter condition is converted to the new structure. The precise + * behaviour of how the old category and tag conditions are converted are covered in + * {@see qbank_managecategories\category_condition_test} and + * {@see qbank_tagquestion\tag_condition_test} respectively. + * * @covers \restore_question_set_reference_data_trait::process_question_set_reference */ public function test_pre_43_quiz_restore_for_random_question_filtercondition(): void { @@ -810,7 +811,6 @@ final class quiz_question_restore_test extends \advanced_testcase { $this->assertArrayHasKey('qperpage', $filterconditions); $this->assertArrayHasKey('filter', $filterconditions); $this->assertArrayHasKey('category', $filterconditions['filter']); - $this->assertArrayHasKey('qtagids', $filterconditions['filter']); $this->assertArrayHasKey('filteroptions', $filterconditions['filter']['category']); $this->assertArrayHasKey('includesubcategories', $filterconditions['filter']['category']['filteroptions']); @@ -819,9 +819,6 @@ final class quiz_question_restore_test extends \advanced_testcase { $this->assertArrayNotHasKey('questioncategoryid', $filterconditions); $this->assertArrayNotHasKey('tags', $filterconditions); - $expectedtags = \core_tag_tag::get_by_name_bulk(1, ['foo', 'bar']); - $expectedtagids = array_values(array_map(fn($expectedtag) => $expectedtag->id, $expectedtags)); - $this->assertEquals($expectedtagids, $filterconditions['filter']['qtagids']['values']); $expectedcategory = $DB->get_record('question_categories', ['idnumber' => 'RAND']); $this->assertEquals($expectedcategory->id, $filterconditions['filter']['category']['values'][0]); $expectedcat = implode(',', [$expectedcategory->id, $expectedcategory->contextid]); diff --git a/question/bank/managecategories/classes/category_condition.php b/question/bank/managecategories/classes/category_condition.php index eafc71cba61..aa28e7dc2a9 100644 --- a/question/bank/managecategories/classes/category_condition.php +++ b/question/bank/managecategories/classes/category_condition.php @@ -16,9 +16,12 @@ namespace qbank_managecategories; +use core\context; use core\output\datafilter; use core_question\local\bank\condition; use core_question\local\bank\view; +use restore_questions_activity_structure_step; +use stdClass; /** * This class controls from which category questions are listed. @@ -303,4 +306,43 @@ class category_condition extends condition { } + #[\Override] + public function restore_filtercondition( + array $filtercondition, + stdClass $setreference, + restore_questions_activity_structure_step $restorestep, + ): array { + global $DB; + // Map category id used for category filter condition and corresponding context id. + $oldcategoryid = $filtercondition['filter']['category']['values'][0]; + // Decide if we're going to refer back to the original category, or to the new category. + // Are we restoring to a different site? + // Has the original context or category been deleted? + // Did the old category belong to the same context as the original set reference? + // Are we allowed to use its questions? + $questionscontext = context::instance_by_id($setreference->questionscontextid, IGNORE_MISSING); + if ( + !$restorestep->get_task()->is_samesite() + || !$questionscontext + || !$DB->record_exists('question_categories', ['id' => $oldcategoryid]) + || $setreference->usingcontextid == $setreference->questionscontextid + || !has_capability('moodle/question:useall', $questionscontext) + ) { + $newcategoryid = $restorestep->get_mappingid('question_category', $oldcategoryid); + $filtercondition['filter']['category']['values'][0] = $newcategoryid; + // Make sure the questions context matches the new category. + $setreference->questionscontextid = $DB->get_field('question_categories', 'contextid', ['id' => $newcategoryid]); + } + + $filtercondition['cat'] = implode( + ',', + [ + $filtercondition['filter']['category']['values'][0], + $setreference->questionscontextid, + ], + ); + + return $filtercondition; + } + } diff --git a/question/bank/managecategories/tests/category_condition_test.php b/question/bank/managecategories/tests/category_condition_test.php new file mode 100644 index 00000000000..7e13c0c9526 --- /dev/null +++ b/question/bank/managecategories/tests/category_condition_test.php @@ -0,0 +1,290 @@ +. + +namespace qbank_managecategories; + +use core\context\module; +use core_question\category_manager; +use core_question\test\mock_restore_test_trait; + +/** + * Unit tests for category_condition + * + * @package qbank_managecategories + * @copyright 2025 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 + * @covers \qbank_managecategories\category_condition + */ +final class category_condition_test extends \advanced_testcase { + use mock_restore_test_trait; + + /** + * Restore a filter where we should keep the reference to the original context. + * + * - We are restoring to the same site. + * - The question context exists. + * - The question category exists. + * - The using context and questions context are different (e.g. the quiz is referencing a category in a qbank). + * - The current user is allowed to use the questions. + */ + public function test_restore_filtercondition(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'values' => [ + $category->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid + 1, + ]; + $mappedid = $category->id + 1; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($mappedid); + + $this->setAdminUser(); + + $condition = new category_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals($category->id, $filtercondition['filter']['category']['values'][0]); + } + + /** + * As {@see test_restore_filtercondition}, but the user does not have permission to use questions in the original category. + */ + public function test_restore_filtercondition_no_permissions(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'values' => [ + $category->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid + 1, + ]; + $mappedid = $category->id + 1; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($mappedid); + + $condition = new category_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals($mappedid, $filtercondition['filter']['category']['values'][0]); + } + + /** + * As {@see test_restore_filtercondition}, but the questions context and using context are the same. + */ + public function test_restore_filtercondition_same_context(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'values' => [ + $category->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid, + ]; + $mappedid = $category->id + 1; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($mappedid); + + $this->setAdminUser(); + + $condition = new category_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals($mappedid, $filtercondition['filter']['category']['values'][0]); + } + + /** + * As {@see test_restore_filtercondition}, but the original category has been deleted. + */ + public function test_restore_filtercondition_no_category(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'values' => [ + $category->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid, + ]; + $mappedid = $category->id + 1; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($mappedid); + + $this->setAdminUser(); + + $manager = new category_manager(); + $manager->delete_category($category->id); + + $condition = new category_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals($mappedid, $filtercondition['filter']['category']['values'][0]); + } + + /** + * As {@see test_restore_filtercondition}, but the original context has been deleted. + */ + public function test_restore_filtercondition_no_context(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + $course = $this->getDataGenerator()->create_course(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $qbank1 = $this->getDataGenerator()->create_module('qbank', ['course' => $course->id]); + $qbank1context = module::instance($qbank1->cmid); + $oldparent = question_get_default_category($qbank1context->id); + $category = $questiongenerator->create_question_category(['parent' => $oldparent->id]); + // Move the category to a different context. + $qbank2 = $this->getDataGenerator()->create_module('qbank', ['course' => $course->id]); + $qbank2context = module::instance($qbank2->cmid); + $newparent = question_get_default_category($qbank2context->id); + $manager = new category_manager(); + $manager->update_category($category->id, "{$newparent->id},{$qbank2context->id}", $category->name, $category->info); + // Delete the original context. + course_delete_module($qbank1->cmid); + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'values' => [ + $category->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid + 1, + ]; + $mappedid = $category->id + 1; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($mappedid); + + $condition = new category_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals($mappedid, $filtercondition['filter']['category']['values'][0]); + } + + /** + * As {@see test_restore_filtercondition}, but restoring to a different site. + */ + public function test_restore_filtercondition_different_site(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'values' => [ + $category->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid + 1, + ]; + $mappedid = $category->id + 1; + $mockstep = $this->get_mock_step($this->get_not_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($mappedid); + + $this->setAdminUser(); + + $condition = new category_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals($mappedid, $filtercondition['filter']['category']['values'][0]); + } + + /** + * When the category is updated in the filter condition, the context ID is also updated. + */ + public function test_restore_filtercondition_update_context(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $fakecategory = $category->id + 1; + $fakecontext = $category->contextid + 1; + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'values' => [ + $fakecategory, + ], + ], + ], + 'cat' => [ + "{$fakecategory},{$fakecontext}", + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $fakecontext, + 'usingcontextid' => $category->contextid, + ]; + $mappedid = $category->id; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($mappedid); + + $this->setAdminUser(); + + $condition = new category_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals($mappedid, $filtercondition['filter']['category']['values'][0]); + $this->assertEquals($category->contextid, $setreference->questionscontextid); + $this->assertEquals($filtercondition['cat'], "{$mappedid},{$category->contextid}"); + } +} diff --git a/question/bank/tagquestion/classes/tag_condition.php b/question/bank/tagquestion/classes/tag_condition.php index 565b7a888dd..02c6a3f3c9c 100644 --- a/question/bank/tagquestion/classes/tag_condition.php +++ b/question/bank/tagquestion/classes/tag_condition.php @@ -18,6 +18,9 @@ namespace qbank_tagquestion; use core\output\datafilter; use core_question\local\bank\condition; +use core_tag_tag; +use restore_questions_activity_structure_step; +use stdClass; /** * Question bank search class to allow searching/filtering by tags on a question. @@ -158,4 +161,35 @@ class tag_condition extends condition { } return $values; } + + #[\Override] + public function restore_filtercondition( + array $filtercondition, + stdClass $setreference, + restore_questions_activity_structure_step $restorestep, + ): array { + if (isset($filtercondition['filter']['qtagids'])) { + $newtagids = []; + foreach ($filtercondition['filter']['qtagids']['values'] as $tagid) { + $tag = core_tag_tag::get($tagid, 'id, name'); + if ($restorestep->get_task()->is_samesite() && $tag) { + $newtagids[] = $tagid; + } else { + // If we're on a different site, or the tag id doesn't exist anymore, look for a mapped tag ID. + $newtagid = $restorestep->get_mappingid('tag', $tagid); + if ($newtagid) { + // Include the new tag. If it wasn't found, we leave it out of the filter. + $newtagids[] = $newtagid; + } + } + } + // Set the filter to the new list of tags. If there were no matching tags found, remove the tags filter. + if (!empty($newtagids)) { + $filtercondition['filter']['qtagids']['values'] = $newtagids; + } else { + unset($filtercondition['filter']['qtagids']); + } + } + return $filtercondition; + } } diff --git a/question/bank/tagquestion/tests/tag_condition_test.php b/question/bank/tagquestion/tests/tag_condition_test.php index 67f0ceb6547..549149a0419 100644 --- a/question/bank/tagquestion/tests/tag_condition_test.php +++ b/question/bank/tagquestion/tests/tag_condition_test.php @@ -18,6 +18,7 @@ namespace qbank_tagquestion; use core\output\datafilter; use core_question\local\bank\question_edit_contexts; +use core_question\test\mock_restore_test_trait; use context_module; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\CoversClass; @@ -35,6 +36,8 @@ use PHPUnit\Framework\Attributes\DataProvider; #[CoversMethod(\qbank_tagquestion\tag_condition::class, 'build_query_from_filter')] #[CoversMethod(\qbank_tagquestion\tag_condition::class, 'get_condition_key')] final class tag_condition_test extends \advanced_testcase { + use mock_restore_test_trait; + /** * Create test environment with questions and tags. * @@ -351,4 +354,171 @@ final class tag_condition_test extends \advanced_testcase { $this->assertSame('', $where); $this->assertSame([], $params); } + + /** + * Restore a filter to the same site, where the tags still exist. The tag IDs remain the same. + */ + public function test_restore_filtercondition(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $tag1 = $this->getDataGenerator()->create_tag(); + $tag2 = $this->getDataGenerator()->create_tag(); + $filtercondition = [ + 'filter' => [ + 'qtagids' => [ + 'values' => [ + $tag1->id, + $tag2->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid, + ]; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($tag2->id + 1); + + $condition = new tag_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals([$tag1->id, $tag2->id], $filtercondition['filter']['qtagids']['values']); + } + + /** + * Restore a filter to a different site, tags should be replaced with mapped IDs. + */ + public function test_restore_filtercondition_different_site(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $tag1 = $this->getDataGenerator()->create_tag(); + $tag2 = $this->getDataGenerator()->create_tag(); + $filtercondition = [ + 'filter' => [ + 'qtagids' => [ + 'values' => [ + $tag1->id, + $tag2->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid, + ]; + $mockstep = $this->get_mock_step($this->get_not_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($tag2->id + 1, $tag2->id + 2); + + $condition = new tag_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals([$tag2->id + 1, $tag2->id + 2], $filtercondition['filter']['qtagids']['values']); + } + + /** + * Restore to the same site where the tag has been deleted, and it's not being restored. It should be removed from the filter. + */ + public function test_restore_filtercondition_deleted_tag(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $tag1 = $this->getDataGenerator()->create_tag(); + $tag2 = $this->getDataGenerator()->create_tag(); + $filtercondition = [ + 'filter' => [ + 'qtagids' => [ + 'values' => [ + $tag1->id, + $tag2->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid, + ]; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + + \core_tag_tag::delete_tags($tag2->id); + + $condition = new tag_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals([$tag1->id], $filtercondition['filter']['qtagids']['values']); + } + + /** + * Restore to the same site where the tag has been deleted, and it is being restored. It should be mapped in the filter. + */ + public function test_restore_filtercondition_restored_tag(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $tag1 = $this->getDataGenerator()->create_tag(); + $tag2 = $this->getDataGenerator()->create_tag(); + $filtercondition = [ + 'filter' => [ + 'qtagids' => [ + 'values' => [ + $tag1->id, + $tag2->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid, + ]; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($tag2->id + 1); + + \core_tag_tag::delete_tags($tag2->id); + + $condition = new tag_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertEquals([$tag1->id, $tag2->id + 1], $filtercondition['filter']['qtagids']['values']); + } + + /** + * Restore to a different site, with no mappings to new IDs. The whole filter should be removed from the filtercondition. + */ + public function test_restore_filtercondition_no_mappings(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $tag1 = $this->getDataGenerator()->create_tag(); + $tag2 = $this->getDataGenerator()->create_tag(); + $filtercondition = [ + 'filter' => [ + 'qtagids' => [ + 'values' => [ + $tag1->id, + $tag2->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid, + ]; + $mockstep = $this->get_mock_step($this->get_not_samesite_task()); + + $condition = new tag_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + + $this->assertArrayNotHasKey('qtagids', $filtercondition['filter']); + } } diff --git a/question/classes/local/bank/condition.php b/question/classes/local/bank/condition.php index f5f1c02a508..8897881d7bc 100644 --- a/question/classes/local/bank/condition.php +++ b/question/classes/local/bank/condition.php @@ -17,6 +17,8 @@ namespace core_question\local\bank; use core\output\datafilter; +use restore_questions_activity_structure_step; +use stdClass; /** * An abstract class for filtering/searching questions. @@ -240,4 +242,20 @@ abstract class condition { * @return array ['SQL where condition', ['param1' => 'value1', 'param2' => 'value2', ...]] */ abstract public static function build_query_from_filter(array $filter): array; + + /** + * Convert and map values in the restored filtercondition to corresponding values on the current site/course. + * + * @param array $filtercondition The origin $filtercondition with preceding conversions applied. + * @param stdClass $setreference The set reference record from the backup. + * @param restore_questions_activity_structure_step $restorestep The restore step. + * @return array the modified $filtercondition + */ + public function restore_filtercondition( + array $filtercondition, + stdClass $setreference, + restore_questions_activity_structure_step $restorestep, + ): array { + return $filtercondition; + } } diff --git a/question/classes/question_reference_manager.php b/question/classes/question_reference_manager.php index 5c1a4e4974c..165c21cc895 100644 --- a/question/classes/question_reference_manager.php +++ b/question/classes/question_reference_manager.php @@ -83,9 +83,11 @@ class question_reference_manager { * pre-4.3 filter condition structure to the new one. * * @param array $filtercondition Pre-4.3 filter condition. + * @param bool $maptags Map tags to tags with the same name, creating them if necessary. If not, just convert existing IDs to + * the new structure. * @return array Post-4.3 filter condition. */ - public static function convert_legacy_set_reference_filter_condition(array $filtercondition): array { + public static function convert_legacy_set_reference_filter_condition(array $filtercondition, bool $maptags = true): array { global $DB; if (!isset($filtercondition['filter'])) { $filtercondition['filter'] = []; @@ -103,10 +105,14 @@ class question_reference_manager { if (isset($filtercondition['tags'])) { // Get the names of the tags in the condition. Find or create corresponding tags, // and set their ids in the new condition. - $oldtags = array_map(fn($oldtag) => explode(',', $oldtag)[1], $filtercondition['tags']); - $questiontagcollid = \core_tag_area::get_collection('core_question', 'question'); - $newtags = \core_tag_tag::create_if_missing($questiontagcollid, $oldtags); - $newtagids = array_map(fn($newtag) => $newtag->id, $newtags); + if ($maptags) { + $oldtags = array_map(fn($oldtag) => explode(',', $oldtag)[1], $filtercondition['tags']); + $questiontagcollid = \core_tag_area::get_collection('core_question', 'question'); + $newtags = \core_tag_tag::create_if_missing($questiontagcollid, $oldtags); + $newtagids = array_map(fn($newtag) => $newtag->id, $newtags); + } else { + $newtagids = array_map(fn($oldtag) => explode(',', $oldtag)[0], $filtercondition['tags']); + } $filtercondition['filter']['qtagids'] = [ 'jointype' => \qbank_tagquestion\tag_condition::JOINTYPE_DEFAULT, diff --git a/question/classes/test/mock_restore_test_trait.php b/question/classes/test/mock_restore_test_trait.php new file mode 100644 index 00000000000..7639eec5c75 --- /dev/null +++ b/question/classes/test/mock_restore_test_trait.php @@ -0,0 +1,71 @@ +. + +namespace core_question\test; + +use PHPUnit\Framework\MockObject\MockObject; + +/** + * Trait for providing mocked question restore objects + * + * @package core_question + * @copyright 2025 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 + */ +trait mock_restore_test_trait { + /** + * Include restore class dependencies. + */ + public static function setUpBeforeClass(): void { + global $CFG; + require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php'); + } + + /** + * Return a mocked restore_task that will say we are restoring to the same site. + * + * @return MockObject The mocked restore_task. + */ + protected function get_samesite_task(): MockObject { + $mock = $this->createMock(\restore_task::class); + $mock->method('is_samesite')->willReturn(true); + return $mock; + } + + /** + * Return a mocked restore_task that will say we are not restoring to the same site. + * + * @return MockObject The mocked restore_task. + */ + protected function get_not_samesite_task(): MockObject { + $mock = $this->createMock(\restore_task::class); + $mock->method('is_samesite')->willReturn(false); + return $mock; + } + + /** + * Restore a mocked restore step that will use the provided task. + * + * @param MockObject $task The mocked restore_task. + * @return MockObject The mocked restore_questions_activity_structure_step. + */ + protected function get_mock_step(MockObject $task): MockObject { + $mock = $this->createMock(\restore_questions_activity_structure_step::class); + $mock->method('get_task')->willReturn($task); + return $mock; + } +}