MDL-67981 questions: avoid errors deleting contextless questions

This commit is contained in:
Tim Hunt
2020-03-06 18:05:33 +00:00
parent 805361d871
commit 0c90bc8cd3
2 changed files with 63 additions and 3 deletions
+14 -3
View File
@@ -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);
+49
View File
@@ -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.
*/