L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-86524 qbank_tagquestion: Map qtagids filter to restored tag IDs

Browse Source 
When restoring a set reference containing a qtagids filter, check that
the tag ID still exists. If not, or we're restoring to a different site,
replace the ID in the filter with the mapped ID from the restore.

If a tag ID is not found, it is removed from the filter. If no matching
tags are found, the entire filter is removed.
This commit is contained in:
Mark Johnson
2026-02-25 09:02:21 +00:00
parent c300ad8d19
commit f4280d2db5
3 changed files with 209 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
public/mod/quiz/tests/quiz_question_restore_test.php
+5 -4
View File
@@ -916,6 +916,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 {
@@ -961,7 +966,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']);
@@ -970,9 +974,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]);
public/question/bank/tagquestion/classes/tag_condition.php
+34
View File
@@ -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;
}
}
public/question/bank/tagquestion/tests/tag_condition_test.php
+170
View File
@@ -19,6 +19,7 @@ namespace qbank_tagquestion;
use core\output\datafilter;
use core_question\local\bank\question_edit_contexts;
use context_module;
use core_question\test\mock_restore_test_trait;
use PHPUnit\Framework\Attributes\CoversMethod;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\DataProvider;
@@ -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']);
}
}