Skip to content

Commit

Permalink
MDL-83977 qbank: Migrate unused question categories
Browse files Browse the repository at this point in the history
Previously, question categories which contained only questions with no
usages were deleted during migration to mod_qbank.

The intention is that hidden questions with no usages should be deleted,
then any categories which are now empty should be deleted rather than
migrated. The case of questions in non-hidden states with no usages was
not covered in the unit tests, and these were being deleted too.

This change will specifically check if a category is empty, rather than
checking question usage, before deletion.
  • Loading branch information
marxjohnson committed Dec 11, 2024
1 parent 0888a6d commit ea34a33
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
2 changes: 1 addition & 1 deletion mod/qbank/classes/task/transfer_question_categories.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public function execute() {
$subcategories = array_reverse(\sort_categories_by_tree($subcategories, $oldtopcategory->id));
foreach ($subcategories as $subcategory) {
\qbank_managecategories\helper::question_remove_stale_questions_from_category($subcategory->id);
if (!question_category_in_use($subcategory->id)) {
if (!$DB->record_exists('question_bank_entries', ['questioncategoryid' => $subcategory->id])) {
question_category_delete_safe($subcategory);
}
}
Expand Down
57 changes: 57 additions & 0 deletions mod/qbank/tests/transfer_question_categories_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ final class transfer_question_categories_test extends \advanced_testcase {
/** @var \core\context\module test quiz mod context */
private \core\context\module $quizcontext;

/** @var \core\context\course Course context with used and unused questions. */
private \core\context\course $usedunusedcontext;

/** @var stdClass[] test stale questions */
private array $stalequestions;

Expand Down Expand Up @@ -236,6 +239,22 @@ protected function setup_pre_install_data(): void {
$question2 = $questiongenerator->create_question('shortanswer', null, ['category' => $quizchildcat1->id]);
quiz_add_quiz_question($question1->id, $quiz, 1);
quiz_add_quiz_question($question2->id, $quiz, 1);

// Set up a course with three categories
// - One contains questions including 1 that is used in a quiz.
// - One contains questions that are not used anywhere, but are in "ready" state.
// - One contains no questions.
$course = self::getDataGenerator()->create_course(['shortname' => 'Used-Unused-Empty']);
$this->usedunusedcontext = context_course::instance($course->id);
$usedcategory = $this->create_question_category(name: 'Used Question Cat', contextid: $this->usedunusedcontext->id);
$unusedcategory = $this->create_question_category('Unused Question Cat', $this->usedunusedcontext->id);
$emptycategory = $this->create_question_category('Empty Cat', $this->usedunusedcontext->id);
$question1 = $questiongenerator->create_question('shortanswer', null, ['category' => $usedcategory->id]);
$question2 = $questiongenerator->create_question('shortanswer', null, ['category' => $usedcategory->id]);
$question3 = $questiongenerator->create_question('shortanswer', null, ['category' => $unusedcategory->id]);
$question4 = $questiongenerator->create_question('shortanswer', null, ['category' => $unusedcategory->id]);
$quiz = $quizgenerator->create_instance(['course' => $course->id, 'grade' => 100.0, 'sumgrades' => 2, 'layout' => '1,0']);
quiz_add_quiz_question($question1->id, $quiz, 1);
}

/**
Expand Down Expand Up @@ -304,6 +323,19 @@ public function test_setup_pre_install_data(): void {
$this->assertEquals($childcat2->id, $grandchildcat1->parent);
$questionids = $this->get_question_data(array_map(static fn($cat) => $cat->id, $questioncats));
$this->assertCount(5, $questionids);

// Make sure the "Used-Unused-Empty" course has 4 question categories (including 'top') with 0, 2, 2, and 0
// questions respectively.
$questioncats = $DB->get_records('question_categories', ['contextid' => $this->usedunusedcontext->id], 'id ASC');
$this->assertCount(4, $questioncats);
$topcat = reset($questioncats);
$this->assertEmpty($this->get_question_data([$topcat->id]));
$usedcat = next($questioncats);
$this->assertCount(2, $this->get_question_data([$usedcat->id]));
$unusedcat = next($questioncats);
$this->assertCount(2, $this->get_question_data([$unusedcat->id]));
$emptycat = next($questioncats);
$this->assertCount(0, $this->get_question_data([$emptycat->id]));
}

/**
Expand Down Expand Up @@ -434,5 +466,30 @@ public function test_qbank_install(): void {
$this->assertCount(3, $quizcategories);
$questions = $this->get_question_data(array_map(static fn($cat) => $cat->id, $quizcategories));
$this->assertCount(2, $questions);

// Used-Unused-Empty checks.
// The empty category should have been removed. The other categories should both have been migrated to a qbank module,
// with all of their questions.
$usedunusedmodinfo = get_fast_modinfo($this->usedunusedcontext->instanceid);
$usedunusedcourse = $usedunusedmodinfo->get_course();
$usedunusedqbanks = $usedunusedmodinfo->get_instances_of('qbank');
$usedunusedqbank = reset($usedunusedqbanks);
$this->assertEquals("{$usedunusedcourse->shortname} shared question bank", $usedunusedqbank->name);

// We should now only have 3 categories. Top, used and unused.
$usedunusedcats = $DB->get_records(
'question_categories',
['contextid' => $usedunusedqbank->context->id],
fields: 'name, id',
);
$this->assertCount(3, $usedunusedcats);
$this->assertTrue(array_key_exists('top', $usedunusedcats));
$this->assertTrue(array_key_exists('Used Question Cat', $usedunusedcats));
$this->assertTrue(array_key_exists('Unused Question Cat', $usedunusedcats));
$this->assertFalse(array_key_exists('Empty Question Cat', $usedunusedcats));

$this->assertEmpty($this->get_question_data([$usedunusedcats['top']->id]));
$this->assertCount(2, $this->get_question_data([$usedunusedcats['Used Question Cat']->id]));
$this->assertCount(2, $this->get_question_data([$usedunusedcats['Unused Question Cat']->id]));
}
}

0 comments on commit ea34a33

Please sign in to comment.