Skip to content

Commit

Permalink
MDL-79316 Deleting Draft questions can break quiz
Browse files Browse the repository at this point in the history
On the missing question placeholder using time() for the prepend part of questionid so it doesn't convert to a string.
Removed the exclusion of draft status from the question with references query to take them into account.
Additional check on the question capability to take into account missing questions.
Unit test
  • Loading branch information
Benjamin-unige committed Feb 16, 2024
1 parent 0f8e328 commit 6356bfe
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 3 deletions.
7 changes: 7 additions & 0 deletions lib/questionlib.php
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,13 @@ function question_has_capability_on($questionorid, $cap, $notused = -1): bool {

// Well, at least we tried. Seems that we really have to read from DB.
$question = $DB->get_record_sql($sql, ['id' => $questionid]);

// The question does not exist, we return false here to exit
// this function and be able to treat it as a missing question
// further down the line.
if ($question === false) {
return false;
}
}
}

Expand Down
33 changes: 33 additions & 0 deletions lib/tests/questionlib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,39 @@ public function test_question_delete_question() {
$this->assertTrue($DB->record_exists('question', ['id' => $q2->id]));
}

/**
* Test that a question with a 'draft' status used in a quiz
* is hidden instead of deleted.
*
* @covers ::question_delete_question
*/
public function test_question_delete_question_used_draft() {
global $DB;
$this->resetAfterTest(true);

list($category, $course, $quiz, $qcat, $questions) = $this->setup_quiz_and_questions();

$q1 = $questions[0];

// Set the status of the question to 'draft'.
$DB->set_field(
'question_versions',
'status',
\core_question\local\bank\question_version_status::QUESTION_STATUS_DRAFT,
['questionid' => $q1->id]
);

// Do.
question_delete_question($q1->id);

// Verify that the question is still existing and that the version status is 'hidden'.
$this->assertTrue($DB->record_exists('question', ['id' => $q1->id]));
$this->assertEquals(
$DB->get_record('question_versions', ['questionid' => $q1->id])->status,
\core_question\local\bank\question_version_status::QUESTION_STATUS_HIDDEN
);
}

/**
* Test that deleting a broken question from the question bank does not cause fatal errors.
*/
Expand Down
2 changes: 1 addition & 1 deletion mod/quiz/classes/question/bank/qbank_helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public static function get_question_structure(int $quizid, \context_module $quiz
$slot->length = 1;
} else if ($slot->qtype === null) {
// This question must have gone missing. Put in a placeholder.
$slot->questionid = 's' . $slot->id; // Sometimes this is used as an array key, so needs to be unique.
$slot->questionid = time() . $slot->id; // Sometimes this is used as an array key, so needs to be unique.
$slot->category = 0;
$slot->qtype = 'missingtype';
$slot->name = get_string('missingquestion', 'quiz');
Expand Down
3 changes: 1 addition & 2 deletions question/classes/question_reference_manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,13 @@ public static function questions_with_references(array $questionids): array {
FROM {question_versions} lqv
JOIN {question_versions} lv ON lv.questionbankentryid = lqv.questionbankentryid
WHERE lqv.questionid $lqidtest
AND lv.status <> :draft
GROUP BY lqv.questionbankentryid
) latestversions ON latestversions.questionbankentryid = qv.questionbankentryid
JOIN {question_references} qr ON qr.questionbankentryid = qv.questionbankentryid
AND (qr.version = qv.version OR qr.version IS NULL AND qv.version = latestversions.latestusableversion)
WHERE qv.questionid $qidtest
", array_merge($params, $lparams, ['draft' => question_version_status::QUESTION_STATUS_DRAFT]));
", array_merge($params, $lparams));
}
}

0 comments on commit 6356bfe

Please sign in to comment.