This commit is contained in:
Huong Nguyen 2025-03-19 10:13:21 +07:00
commit 341a42df72
10 changed files with 125 additions and 40 deletions

View File

@ -218,6 +218,7 @@ $string['invalidcontextinhasanyquestions'] = 'Invalid context passed to question
$string['invalidgrade'] = 'Grades ({$a}) do not match grade options - question skipped.';
$string['invalidgradequestion'] = 'Grades ({$a->grades}) do not match grade options - question \'{$a->question}\' skipped.';
$string['invalidpenalty'] = 'Invalid penalty';
$string['invalidquestiontype'] = 'Invalid question type: {$a}';
$string['invalidwizardpage'] = 'Incorrect or no wizard page specified!';
$string['lastmodifiedby'] = 'Last modified by';
$string['lasttry'] = 'Last try';

View File

@ -923,6 +923,11 @@ 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)) {
return '';
}
if (isset($question->questionid)) {
$question->id = $question->questionid;

View File

@ -52,6 +52,9 @@ class preview_action_column extends \core_question\local\bank\column_base {
if (!question_has_capability_on($question, 'use')) {
return;
}
if (!\question_bank::is_qtype_usable($question->qtype)) {
return;
}
$editrenderer = $PAGE->get_renderer('quiz', 'edit');
echo $editrenderer->question_preview_icon($this->qbank->get_quiz(), $question);
}

View File

@ -109,24 +109,12 @@ function qbank_comment_preview_display($question, $courseid): string {
* @return string rendered output
*/
function qbank_comment_output_fragment_question_comment($args): string {
global $USER, $PAGE, $CFG, $DB;
global $PAGE, $CFG;
$displaydata = [];
require_once($CFG->dirroot . '/question/engine/bank.php');
$question = question_bank::load_question($args['questionid']);
$quba = question_engine::make_questions_usage_by_activity(
'core_question_preview', context_user::instance($USER->id));
$displaydata['question'] = question_bank::render_preview_of_question($question);
// Just in case of any regression, it should not break the modal, just show the comments.
if (class_exists('\\qbank_previewquestion\\question_preview_options')) {
$options = new \qbank_previewquestion\question_preview_options($question);
$quba->set_preferred_behaviour($options->behaviour);
$slot = $quba->add_question($question, $options->maxmark);
$quba->start_question($slot, $options->variant);
$transaction = $DB->start_delegated_transaction();
question_engine::save_questions_usage_by_activity($quba);
$transaction->allow_commit();
$displaydata['question'] = $quba->render_question($slot, $options, '1');
}
$displaydata['comment'] = qbank_comment_preview_display($question, $args['courseid']);
$displaydata['commenstdisabled'] = false;
if (empty($displaydata['comment']) && !$CFG->usecomments) {

View File

@ -47,7 +47,8 @@ class question_preview_options extends question_display_options {
/**
* Constructor.
* @param \stdClass $question
*
* @param \stdClass|\question_definition $question only the ->defaultmark field is used.
*/
public function __construct($question) {
$this->behaviour = 'deferredfeedback';

View File

@ -32,21 +32,12 @@ defined('MOODLE_INTERNAL') || die();
* @return string rendered output
*/
function qbank_usage_output_fragment_question_usage(array $args): string {
global $USER, $PAGE, $CFG, $DB;
global $PAGE, $CFG;
require_once($CFG->dirroot . '/question/engine/bank.php');
$displaydata = [];
$question = question_bank::load_question($args['questionid']);
$quba = question_engine::make_questions_usage_by_activity('core_question_preview', context_user::instance($USER->id));
$options = new \qbank_previewquestion\question_preview_options($question);
$quba->set_preferred_behaviour($options->behaviour);
$slot = $quba->add_question($question, $options->maxmark);
$quba->start_question($slot, $options->variant);
$transaction = $DB->start_delegated_transaction();
question_engine::save_questions_usage_by_activity($quba);
$transaction->allow_commit();
$displaydata['question'] = $quba->render_question($slot, $options, '1');
$displaydata['question'] = question_bank::render_preview_of_question($question);
$specificversion = clean_param($args['specificversion'] ?? false, PARAM_BOOL);
$questionusagetable = new \qbank_usage\tables\question_usage_table('question_usage_table', $question, $specificversion);

View File

@ -61,6 +61,13 @@ class question_name_idnumber_tags_column extends viewquestionname_column_helper
}
echo \html_writer::end_tag('div');
// If the question is invalid, show a warning badge.
if (!\question_bank::is_qtype_usable($question->qtype)) {
echo \html_writer::span(get_string('invalidquestiontype', 'question', $question->qtype),
'badge bg-danger text-white');
}
}
public function get_required_fields(): array {

View File

@ -1178,16 +1178,7 @@ class view {
);
echo $this->get_plugin_controls($catcontext, $categoryid);
$this->build_query();
$totalquestions = $this->get_question_count();
$questionsrs = $this->load_page_questions();
$questions = [];
foreach ($questionsrs as $question) {
if (!empty($question->id)) {
$questions[$question->id] = $question;
}
}
$questionsrs->close();
$questions = $this->load_questions();
// This html will be refactored in the bulk actions implementation.
echo \html_writer::start_tag('form', ['action' => $this->baseurl, 'method' => 'post', 'id' => 'questionsubmit']);
@ -1200,7 +1191,7 @@ class view {
// Embeded filterconditon into the div.
echo \html_writer::start_tag('div',
['class' => 'categoryquestionscontainer', 'data-filtercondition' => $filtercondition]);
if ($totalquestions > 0) {
if ($this->totalcount > 0) {
// Bulk load any required statistics.
$this->load_required_statistics($questions);
@ -1423,6 +1414,7 @@ class view {
*/
public function load_questions() {
$this->build_query();
$this->get_question_count();
$questionsrs = $this->load_page_questions();
$questions = [];
foreach ($questionsrs as $question) {
@ -1538,6 +1530,11 @@ class view {
public function print_table_row($question, $rowcount): void {
$rowclasses = implode(' ', $this->get_row_classes($question, $rowcount));
$attributes = [];
// If the question type is invalid we highlight it red.
if (!\question_bank::is_qtype_usable($question->qtype)) {
$rowclasses .= ' table-danger';
}
if ($rowclasses) {
$attributes['class'] = $rowclasses;
}
@ -1628,9 +1625,8 @@ class view {
public function display_questions_table(): string {
$this->add_standard_search_conditions();
$questions = $this->load_questions();
$totalquestions = $this->get_question_count();
$questionhtml = '';
if ($totalquestions > 0) {
if ($this->get_question_count() > 0) {
$this->load_required_statistics($questions);
ob_start();
$this->display_questions($questions, $this->pagevars['qpage'], $this->pagevars['qperpage']);

View File

@ -27,12 +27,13 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
use core\output\notification;
use core_cache\application_cache;
use core_cache\data_source_interface;
use core_cache\definition;
use core_question\local\bank\question_version_status;
use core_question\output\question_version_info;
use qbank_previewquestion\question_preview_options;
defined('MOODLE_INTERNAL') || die();
@ -83,6 +84,23 @@ abstract class question_bank {
return $plugindir && is_readable($plugindir . '/questiontype.php');
}
/**
* Check if a given question type is one that is installed and usable.
*
* Use this before doing things like rendering buttons/options which will only work for
* installed question types.
*
* When loaded through most of the core_question areas, qtype will still be the uninstalled type, e.g. 'mytype',
* but when we get to the quiz pages, it will have been converted to 'missingtype'. So we need to check that
* as well here.
*
* @param string $qtypename e.g. 'multichoice'.
* @return bool
*/
public static function is_qtype_usable(string $qtypename): bool {
return self::is_qtype_installed($qtypename) && $qtypename !== 'missingtype';
}
/**
* Get the question type class for a particular question type.
* @param string $qtypename the question type name. For example 'multichoice' or 'shortanswer'.
@ -296,6 +314,49 @@ abstract class question_bank {
return $definition;
}
/**
* Render a throw-away preview of a question.
*
* If the question cannot be rendered (e.g. because it is not installed)
* then display a message instead.
*
* @param question_definition $question a question.
* @return string HTML to output.
*/
public static function render_preview_of_question(question_definition $question): string {
global $DB, $OUTPUT, $USER;
if (!self::is_qtype_usable($question->qtype->name())) {
// TODO MDL-84902 ideally this would be changed to render at least the qeuestion text.
// See, for example, test_render_missing in question/type/missingtype/tests/missingtype_test.php.
return $OUTPUT->notification(
get_string('invalidquestiontype', 'question', $question->qtype->name()),
notification::NOTIFY_WARNING,
closebutton: false);
}
// TODO MDL-84902 remove this dependency on a class from qbank_previewquestion plugin.
if (!class_exists(question_preview_options::class)) {
debugging('Preview cannot be rendered. The standard plugin ' .
'qbank_previewquestion plugin has been removed.', DEBUG_DEVELOPER);
return '';
}
$quba = question_engine::make_questions_usage_by_activity(
'core_question_preview', context_user::instance($USER->id));
$options = new question_preview_options($question);
$quba->set_preferred_behaviour($options->behaviour);
$slot = $quba->add_question($question, $options->maxmark);
$quba->start_question($slot, $options->variant);
$transaction = $DB->start_delegated_transaction();
question_engine::save_questions_usage_by_activity($quba);
$transaction->allow_commit();
return $quba->render_question($slot, $options, '1');
}
/**
* Get all the versions of a particular question.
*

View File

@ -0,0 +1,32 @@
@qtype @qtype_missingtype
Feature: Questions with invalid types should be clear and any actions which won't work should be disabled
As a teacher
In order to manage my questions
I want to be able to clearly see which are invalid
Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
And the following "courses" exist:
| fullname | shortname | format |
| Course 1 | C1 | weeks |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
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 | teacher1 | A text |
| Test questions | essay | Question 2 | teacher1 | B text |
And question "Question 2" is changed to simulate being of an uninstalled type
Scenario: Questions of invalid types should be highlighted and labelled as invalid
Given I am on the "Qbank 1" "core_question > question bank" page logged in as "teacher1"
Then I should see "Invalid question type: invalidqtype" in the table row containing "Question 2"
And "//tr[contains(., 'Question 2')][contains(@class, 'table-danger')]" "xpath_element" should exist