MDL-84302 mod_quiz: Improvements with invalid questions.

- Improve visuals of invalid questions by highlighting and
labelling.
- Removes links to actions which will not work for invalid
questions
- Doesn't allow a preview/attempt of a quiz if it has invalid
questions. This is an improvement over it throwing an exception.
- Don't allow the adding of questions to a quiz if they have
an invalid type.
This commit is contained in:
Conn Warwicker
2025-03-19 18:42:50 +00:00
committed by Tim Hunt
parent e4c7a5cb64
commit 2d6df85dea
9 changed files with 123 additions and 16 deletions
+21 -9
View File
@@ -28,6 +28,7 @@ use core_question\local\bank\question_version_status;
use \mod_quiz\structure;
use \html_writer;
use qbank_previewquestion\question_preview_options;
use question_bank;
use renderable;
/**
@@ -867,7 +868,7 @@ class edit_renderer extends \plugin_renderer_base {
$qtype = $structure->get_question_type_for_slot($slot);
$slotinfo = $structure->get_slot_by_number($slot);
$questionicons = '';
if ($qtype !== 'random') {
if ($qtype !== 'random' && question_bank::is_qtype_usable($qtype)) {
$questionicons .= $this->question_preview_icon($structure->get_quiz(),
$structure->get_question_in_slot($slot),
null, null, $slotinfo->requestedversion ?: question_preview_options::ALWAYS_LATEST);
@@ -924,7 +925,7 @@ class edit_renderer extends \plugin_renderer_base {
public function question_preview_icon($quiz, $questiondata, $label = null, $variant = null, $restartversion = null) {
$question = clone($questiondata);
if (!\question_bank::is_qtype_usable($question->qtype)) {
if (!question_bank::is_qtype_usable($question->qtype)) {
return '';
}
@@ -1062,7 +1063,7 @@ class edit_renderer extends \plugin_renderer_base {
$instancename = quiz_question_tostring($question);
$qtype = \question_bank::get_qtype($question->qtype, false);
$qtype = question_bank::get_qtype($question->qtype, false);
$namestr = $qtype->local_name();
$icon = $this->pix_icon('icon', $namestr, $qtype->plugin_name(), ['title' => $namestr,
@@ -1073,10 +1074,21 @@ class edit_renderer extends \plugin_renderer_base {
// Need plain question name without html tags for link title.
$title = shorten_text(format_string($question->name), 100);
// Display the link itself.
$activitylink = $icon . html_writer::tag('span', $editicon . $instancename, ['class' => 'instancename']);
$output .= html_writer::link($editurl, $activitylink,
['title' => get_string('editquestion', 'quiz').' '.$title]);
// If the question is invalid, don't show the link as it won't work.
if (!question_bank::is_qtype_usable($question->qtype)) {
$output .= html_writer::span($title);
$output .= html_writer::span(
get_string('invalidquestiontype', 'question', $question->originalqtype),
'badge bg-danger text-white ml-3'
);
} else {
// Display the link itself.
$activitylink = $icon . html_writer::tag('span', $editicon . $instancename, ['class' => 'instancename']);
$output .= html_writer::link($editurl, $activitylink,
['title' => get_string('editquestion', 'quiz') . ' ' . $title]);
}
return $output;
}
@@ -1110,7 +1122,7 @@ class edit_renderer extends \plugin_renderer_base {
}
$configuretitle = get_string('configurerandomquestion', 'quiz');
$qtype = \question_bank::get_qtype($question->qtype, false);
$qtype = question_bank::get_qtype($question->qtype, false);
$namestr = $qtype->local_name();
$icon = $this->pix_icon('icon', $namestr, $qtype->plugin_name(), ['class' => 'icon activityicon']);
@@ -1294,7 +1306,7 @@ class edit_renderer extends \plugin_renderer_base {
'questiondependsonprevious',
], 'quiz');
foreach (\question_bank::get_all_qtypes() as $qtype => $notused) {
foreach (question_bank::get_all_qtypes() as $qtype => $notused) {
$this->page->requires->string_for_js('pluginname', 'qtype_' . $qtype);
}
@@ -189,6 +189,7 @@ class qbank_helper {
$slot->length = 1;
} else if (!\question_bank::qtype_exists($slot->qtype)) {
// Question of unknown type found in the database. Set to placeholder question types instead.
$slot->originalqtype = $slot->qtype;
$slot->qtype = 'missingtype';
} else {
$slot->_partiallyloaded = 1;
+1
View File
@@ -807,6 +807,7 @@ $string['quiz:emailnotifyattemptgraded'] = 'Receive notification when your attem
$string['quiz:emailwarnoverdue'] = 'Receive warning when your quiz attempt becomes overdue';
$string['quiz:grade'] = 'Grade quizzes manually';
$string['quiz:ignoretimelimits'] = 'Ignore quiz time limit';
$string['quizinvalidquestions'] = 'This quiz has questions with invalid types. The missing question type must be reinstalled or the affected questions removed, before the quiz can be used.';
$string['quizisclosed'] = 'This quiz is closed';
$string['quizisopen'] = 'This quiz is open';
$string['quizisclosedwillopen'] = 'Quiz closed (opens {$a})';
+6
View File
@@ -1714,6 +1714,12 @@ function quiz_add_quiz_question($questionid, $quiz, $page = 0, $maxmark = null)
);
}
// If the question type is invalid, we cannot add it to the quiz. It shouldn't be possible to get to this
// point without fiddling with the DOM so we can just throw an exception.
if (!\question_bank::is_qtype_installed($questiontype)) {
throw new coding_exception('Invalid question type: ' . $questiontype);
}
$trans = $DB->start_delegated_transaction();
$sql = "SELECT qbe.id
+8
View File
@@ -644,6 +644,14 @@ table.quizreviewsummary td.cell {
position: relative;
}
#page-mod-quiz-edit ul.slots li.section li.activity.qtype_missingtype {
background-color: #f0c5c1;
}
#page-mod-quiz-edit ul.slots li.section li.activity.qtype_missingtype .actions {
background-color: inherit;
}
#page-mod-quiz-edit ul.slots li.section li.activity.page {
background: transparent;
}
@@ -0,0 +1,41 @@
@mod_quiz
Feature: A quiz with invalid question types should not be able to be attempted
As a teacher
If my quiz has questions with invalid types
I want my students to be unable to attempt the quiz until it is fixed
Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| student | Student | One | student@example.com |
And the following "courses" exist:
| fullname | shortname | category |
| Course 1 | C1 | 0 |
And the following "course enrolments" exist:
| user | course | role |
| student | C1 | student |
And the following "activities" exist:
| activity | name | intro | course | idnumber |
| qbank | Qbank 1 | Question bank 1 | C1 | qbank1 |
And the following "question categories" exist:
| contextlevel | reference | name |
| Activity module | qbank1 | Test questions |
And the following "questions" exist:
| questioncategory | qtype | name | user | questiontext |
| Test questions | essay | Question 1 | admin | A text |
| Test questions | essay | Question 2 | admin | B text |
And the following "activities" exist:
| activity | name | intro | course | idnumber | grade | navmethod |
| quiz | Quiz 1 | Quiz 1 description | C1 | quiz1 | 100 | free |
And quiz "Quiz 1" contains the following questions:
| question | page | maxmark |
| Question 1 | 1 | |
| Question 2 | 1 | |
And question "Question 2" is changed to simulate being of an uninstalled type
@javascript
Scenario: Quiz with invalid questions should disable attempts
Given I am logged in as "student"
When I am on the "Quiz 1" "mod_quiz > View" page
Then I should see "This quiz has questions with invalid types"
And I should not see "Attempt quiz"
+9 -1
View File
@@ -23,6 +23,7 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
use core\output\notification;
use mod_quiz\access_manager;
use mod_quiz\output\list_of_attempts;
use mod_quiz\output\renderer;
@@ -261,6 +262,13 @@ if (!$viewobj->quizhasquestions) {
}
}
}
// If the quiz has any invalid questions, we cannot attempt it.
if (in_array('missingtype', $quizobj->get_all_question_types_used())) {
$viewobj->preventmessages[] = $OUTPUT->notification(
get_string('quizinvalidquestions', 'mod_quiz'), notification::NOTIFY_ERROR, false);
$viewobj->buttontext = '';
}
}
$viewobj->showbacktocourse = ($viewobj->buttontext === '' &&
@@ -270,7 +278,7 @@ echo $OUTPUT->header();
if (!empty($gradinginfo->errors)) {
foreach ($gradinginfo->errors as $error) {
$errortext = new \core\output\notification($error, \core\output\notification::NOTIFY_ERROR);
$errortext = new notification($error, notification::NOTIFY_ERROR);
echo $OUTPUT->render($errortext);
}
}
@@ -46,8 +46,10 @@ class random_question_loader {
/** @var \qubaid_condition which usages to consider previous attempts from. */
protected $qubaids;
/** @var array qtypes that cannot be used by random questions. */
protected $excludedqtypes;
/**
* @var array Array of question types to include in random questions.
*/
protected $includedqtypes = [];
/** @var array categoryid & include subcategories => num previous uses => questionid => 1. */
protected $availablequestionscache = [];
@@ -69,9 +71,10 @@ class random_question_loader {
$this->qubaids = $qubaids;
$this->recentlyusedquestions = $usedquestions;
// Load the possible question types we can select from.
foreach (\question_bank::get_all_qtypes() as $qtype) {
if (!$qtype->is_usable_by_random()) {
$this->excludedqtypes[] = $qtype->name();
if ($qtype->is_usable_by_random()) {
$this->includedqtypes[] = $qtype->name();
}
}
}
@@ -213,8 +216,8 @@ class random_question_loader {
$filtercondition = $filterconditions ? 'AND ' . implode(' AND ', $filterconditions) : '';
// Prepare qtype check.
[$qtypecondition, $qtypeparams] = $DB->get_in_or_equal($this->excludedqtypes,
SQL_PARAMS_NAMED, 'excludedqtype', false);
[$qtypecondition, $qtypeparams] = $DB->get_in_or_equal($this->includedqtypes,
SQL_PARAMS_NAMED, 'includedqtype');
if ($qtypecondition) {
$qtypecondition = 'AND q.qtype ' . $qtypecondition;
}
@@ -680,4 +680,31 @@ final class random_question_loader_test extends \advanced_testcase {
return [$category, $questions];
}
/**
* Test that the random question loader excludes questions with invalid types.
* @return void
* @throws \dml_exception
*/
public function test_invalid_questions_are_excluded(): void {
global $DB;
$this->resetAfterTest();
[$category, $questions] = $this->create_category_and_questions(4);
$loader = new random_question_loader(new qubaid_list([]));
$filters = question_filter_test_helper::create_filters([$category->id], false, []);
// Update one of the questions to have an invalid type.
$invalid = $questions[0];
$invalid->qtype = 'invalid';
$DB->update_record('question', $invalid);
// Assert that we get 1 less result back because the invalid type is excluded.
$result = $loader->get_filtered_questions($filters);
$this->assertEquals(count($questions) - 1, count($result));
}
}