diff --git a/question/bank/managecategories/category.php b/question/bank/managecategories/category.php index 183fff8f942..ade905f02ec 100644 --- a/question/bank/managecategories/category.php +++ b/question/bank/managecategories/category.php @@ -107,7 +107,7 @@ if ($param->delete) { helper::question_remove_stale_questions_from_category($param->delete); - $questionstomove = $DB->count_records('question_bank_entries', ['questioncategoryid' => $param->delete]); + $questionstomove = count($qcobject->get_real_question_ids_in_category($param->delete)); // Second pass, if we still have questions to move, setup the form. if ($questionstomove) { diff --git a/question/bank/managecategories/classes/question_category_object.php b/question/bank/managecategories/classes/question_category_object.php index 39e6d561cc5..1fae4063903 100644 --- a/question/bank/managecategories/classes/question_category_object.php +++ b/question/bank/managecategories/classes/question_category_object.php @@ -324,17 +324,8 @@ class question_category_object { * @throws \dml_exception */ public function move_questions(int $oldcat, int $newcat): void { - global $DB; - - $sql = "SELECT q.id, 1 - FROM {question} q - JOIN {question_versions} qv ON qv.questionid = q.id - JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid - WHERE qbe.questioncategoryid = ? - AND (q.parent = 0 OR q.parent = q.id)"; - - $questionids = $DB->get_records_sql_menu($sql, [$oldcat]); - question_move_questions_to_category(array_keys($questionids), $newcat); + $questionids = $this->get_real_question_ids_in_category($oldcat); + question_move_questions_to_category($questionids, $newcat); } /** @@ -516,4 +507,27 @@ class question_category_object { redirect($this->pageurl); } } + + /** + * Returns ids of the question in the given question category. + * + * This method only returns the real question. It does not include + * subquestions of question types like multianswer. + * + * @param int $categoryid id of the category. + * @return int[] array of question ids. + */ + public function get_real_question_ids_in_category(int $categoryid): array { + global $DB; + + $sql = "SELECT q.id + FROM {question} q + JOIN {question_versions} qv ON qv.questionid = q.id + JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid + WHERE qbe.questioncategoryid = :categoryid + AND (q.parent = 0 OR q.parent = q.id)"; + + $questionids = $DB->get_records_sql($sql, ['categoryid' => $categoryid]); + return array_keys($questionids); + } } diff --git a/question/bank/managecategories/tests/behat/question_categories.feature b/question/bank/managecategories/tests/behat/question_categories.feature index e2b8e7c7bd5..9c32b807fdd 100644 --- a/question/bank/managecategories/tests/behat/question_categories.feature +++ b/question/bank/managecategories/tests/behat/question_categories.feature @@ -71,3 +71,20 @@ Feature: A teacher can put questions in categories in the question bank Then I should not see "Used category" And I follow "Add category" And I should see "Default for C1 (1)" + + @_file_upload + Scenario: Multi answer questions with their child questions can be moved to another category when the current category is deleted + When I navigate to "Question bank" in current page administration + And I select "Import" from the "Question bank tertiary navigation" singleselect + And I set the field "id_format_xml" to "1" + And I upload "question/format/xml/tests/fixtures/multianswer.xml" file to "Import" filemanager + And I press "id_submitbutton" + And I press "Continue" + And I select "Categories" from the "Question bank tertiary navigation" singleselect + And I click on "Delete" "link" in the "Default for Test images in backup" "list_item" + And I should see "The category 'Default for Test images in backup' contains 1 questions" + And I select "Used category" from the "Category" singleselect + And I press "Save in category" + Then I should not see "Default for Test images in backup" + And I follow "Add category" + And I should see "Used category (2)" diff --git a/question/bank/managecategories/tests/question_category_object_test.php b/question/bank/managecategories/tests/question_category_object_test.php index ed889290982..8b6efbaacd7 100644 --- a/question/bank/managecategories/tests/question_category_object_test.php +++ b/question/bank/managecategories/tests/question_category_object_test.php @@ -340,4 +340,100 @@ class question_category_object_test extends \advanced_testcase { $this->assertDebuggingNotCalled(); } + + /** + * Test that get_real_question_ids_in_category() returns question id + * of a shortanswer question in a category. + * + * @covers ::get_real_question_ids_in_category + */ + public function test_get_real_question_ids_in_category_shortanswer() { + $generator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $categoryid = $this->defaultcategoryobj->id; + + // Short answer question is made of one question. + $shortanswer = $generator->create_question('shortanswer', null, ['category' => $categoryid]); + $questionids = $this->qcobject->get_real_question_ids_in_category($categoryid); + $this->assertCount(1, $questionids); + $this->assertContains($shortanswer->id, $questionids); + } + + /** + * Test that get_real_question_ids_in_category() returns question id + * of a multianswer question in a category. + * + * @covers ::get_real_question_ids_in_category + */ + public function test_get_real_question_ids_in_category_multianswer() { + global $DB; + $countq = $DB->count_records('question'); + $countqbe = $DB->count_records('question_bank_entries'); + + $generator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $categoryid = $this->defaultcategoryobj->id; + + // Multi answer question is made of one parent and two child questions. + $multianswer = $generator->create_question('multianswer', null, ['category' => $categoryid]); + $questionids = $this->qcobject->get_real_question_ids_in_category($categoryid); + $this->assertCount(1, $questionids); + $this->assertContains($multianswer->id, $questionids); + $this->assertEquals(3, $DB->count_records('question') - $countq); + $this->assertEquals(3, $DB->count_records('question_bank_entries') - $countqbe); + } + + /** + * Test that get_real_question_ids_in_category() returns question ids + * of two versions of a multianswer question in a category. + * + * @covers ::get_real_question_ids_in_category + */ + public function test_get_real_question_ids_in_category_multianswer_two_versions() { + global $DB; + $countq = $DB->count_records('question'); + $countqv = $DB->count_records('question_versions'); + $countqbe = $DB->count_records('question_bank_entries'); + + $generator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $categoryid = $this->defaultcategoryobj->id; + + // Create two versions of a multianswer question which will lead to + // 2 parents and 4 child questions in the question bank. + $multianswer = $generator->create_question('multianswer', null, ['category' => $categoryid]); + $multianswernew = $generator->update_question($multianswer, null, ['name' => 'This is a new version']); + $questionids = $this->qcobject->get_real_question_ids_in_category($categoryid); + $this->assertCount(2, $questionids); + $this->assertContains($multianswer->id, $questionids); + $this->assertContains($multianswernew->id, $questionids); + $this->assertEquals(6, $DB->count_records('question') - $countq); + $this->assertEquals(6, $DB->count_records('question_versions') - $countqv); + $this->assertEquals(3, $DB->count_records('question_bank_entries') - $countqbe); + } + + /** + * Test that get_real_question_ids_in_category() returns question id + * of a multianswer question in a category even if their child questions are + * linked to a category that doesn't exist. + * + * @covers ::get_real_question_ids_in_category + */ + public function test_get_real_question_ids_in_category_multianswer_bad_data() { + global $DB; + $countqbe = $DB->count_records('question_bank_entries'); + + $generator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $categoryid = $this->defaultcategoryobj->id; + + // Multi answer question is made of one parent and two child questions. + $multianswer = $generator->create_question('multianswer', null, ['category' => $categoryid]); + $qversion = $DB->get_record('question_versions', ['questionid' => $multianswer->id]); + + // Update category id for child questions to a category that doesn't exist. + $DB->set_field_select('question_bank_entries', 'questioncategoryid', + 123456, 'id <> :id', ['id' => $qversion->questionbankentryid]); + + $questionids = $this->qcobject->get_real_question_ids_in_category($categoryid); + $this->assertCount(1, $questionids); + $this->assertContains($multianswer->id, $questionids); + $this->assertEquals(3, $DB->count_records('question_bank_entries') - $countqbe); + } }