diff --git a/lib/questionlib.php b/lib/questionlib.php index e7de6c840c4..e3c0639bd54 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -332,16 +332,18 @@ function question_category_in_use($categoryid, $recursive = false) { /** * Deletes question and all associated data from the database * - * It will not delete a question if it is used by an activity module + * It will not delete a question if it is used somewhere. + * * @param object $question The question being deleted */ function question_delete_question($questionid) { global $DB; $question = $DB->get_record_sql(' - SELECT q.*, qc.contextid + SELECT q.*, ctx.id AS contextid FROM {question} q - JOIN {question_categories} qc ON qc.id = q.category + LEFT JOIN {question_categories} qc ON qc.id = q.category + LEFT JOIN {context} ctx ON ctx.id = qc.contextid WHERE q.id = ?', array($questionid)); if (!$question) { // In some situations, for example if this was a child of a @@ -355,6 +357,15 @@ function question_delete_question($questionid) { return; } + // This sometimes happens in old sites with bad data. + if (!$question->contextid) { + debugging('Deleting question ' . $question->id . ' which is no longer linked to a context. ' . + 'Assuming system context to avoid errors, but this may mean that some data like files, ' . + 'tags, are not cleaned up.'); + $question->contextid = context_system::instance()->id; + } + + // Delete previews of the question. $dm = new question_engine_data_mapper(); $dm->delete_previews($questionid); diff --git a/lib/tests/questionlib_test.php b/lib/tests/questionlib_test.php index 6f295e889a9..cd04aceb11d 100644 --- a/lib/tests/questionlib_test.php +++ b/lib/tests/questionlib_test.php @@ -273,6 +273,55 @@ class core_questionlib_testcase extends advanced_testcase { $rc->destroy(); } + /** + * Test that deleting a question from the question bank works in the normal case. + */ + public function test_question_delete_question() { + global $DB; + + // Setup. + $context = context_system::instance(); + $qgen = $this->getDataGenerator()->get_plugin_generator('core_question'); + $qcat = $qgen->create_question_category(array('contextid' => $context->id)); + $q1 = $qgen->create_question('shortanswer', null, array('category' => $qcat->id)); + $q2 = $qgen->create_question('shortanswer', null, array('category' => $qcat->id)); + + // Do. + question_delete_question($q1->id); + + // Verify. + $this->assertFalse($DB->record_exists('question', ['id' => $q1->id])); + // Check that we did not delete too much. + $this->assertTrue($DB->record_exists('question', ['id' => $q2->id])); + } + + /** + * Test that deleting a broken question from the question bank does not cause fatal errors. + */ + public function test_question_delete_question_broken_data() { + global $DB; + + // Setup. + $context = context_system::instance(); + $qgen = $this->getDataGenerator()->get_plugin_generator('core_question'); + $qcat = $qgen->create_question_category(array('contextid' => $context->id)); + $q1 = $qgen->create_question('shortanswer', null, array('category' => $qcat->id)); + + // Now delete the category, to simulate what happens in old sites where + // referential integrity has failed. + $DB->delete_records('question_categories', ['id' => $qcat->id]); + + // Do. + question_delete_question($q1->id); + + // Verify. + $this->assertDebuggingCalled('Deleting question ' . $q1->id . + ' which is no longer linked to a context. Assuming system context ' . + 'to avoid errors, but this may mean that some data like ' . + 'files, tags, are not cleaned up.'); + $this->assertFalse($DB->record_exists('question', ['id' => $q1->id])); + } + /** * This function tests the question_category_delete_safe function. */