Skip to content

Commit

Permalink
MDL-74255 quiz: handle draft question status correctly
Browse files Browse the repository at this point in the history 
The main issue to fix is that questions vesions which should not have
been used (that is, hidden or draft versions) were getting offered
as an option and acutally being used.

As part of this I was able to substantially un-tangle
mod_quiz\question\bank\qbank_helper, which previously was
a mass of functions calling other functions in a complicated way.
Hopefully, it is now a bit easer to understand, and perhaps
less buggy.
  • Loading branch information
@timhunt
timhunt committed Apr 8, 2022
1 parent 5fff990 commit 839ccce
Show file tree
Hide file tree
Showing 14 changed files with 436 additions and 553 deletions.
7 changes: 5 additions & 2 deletions backup/tests/quiz_restore_decode_links_test.php
View file
Expand Up @@ -80,10 +80,13 @@ public function test_restore_quiz_decode_links() {

$newcm = duplicate_module($course, get_fast_modinfo($course)->get_cm($quiz->cmid));

$quizquestions = \mod_quiz\question\bank\qbank_helper::get_question_structure_data($newcm->instance);
$quizquestions = \mod_quiz\question\bank\qbank_helper::get_question_structure(
$newcm->instance, context_module::instance($newcm->id));
$questionids = [];
foreach ($quizquestions as $quizquestion) {
$questionids[] = $quizquestion->id;
if ($quizquestion->questionid) {
$questionids[] = $quizquestion->questionid;
}
}
list($condition, $param) = $DB->get_in_or_equal($questionids, SQL_PARAMS_NAMED, 'questionid');
$condition = 'WHERE qa.question ' . $condition;
Expand Down
56 changes: 34 additions & 22 deletions mod/quiz/attemptlib.php
View file
Expand Up @@ -25,9 +25,10 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/


defined('MOODLE_INTERNAL') || die();

use mod_quiz\question\bank\qbank_helper;


/**
* Class for quiz exceptions. Just saves a couple of arguments on the
Expand Down Expand Up @@ -79,7 +80,11 @@ class quiz {
/** @var context the quiz context. */
protected $context;

/** @var stdClass[] of questions augmented with slot information. */
/**
* @var stdClass[] of questions augmented with slot information. For non-random
* questions, the array key is question id. For random quesions it is 's' . $slotid.
* probalby best to use ->questionid field of the object instead.
*/
protected $questions = null;
/** @var stdClass[] of quiz_section rows. */
protected $sections = null;
Expand Down Expand Up @@ -145,35 +150,33 @@ public function create_attempt_object($attemptdata) {
* Load just basic information about all the questions in this quiz.
*/
public function preload_questions() {
$specificquestionids = \mod_quiz\question\bank\qbank_helper::get_specific_version_question_ids($this->quiz->id);
$latestquestionids = \mod_quiz\question\bank\qbank_helper::get_always_latest_version_question_ids($this->quiz->id);
$questionids = array_merge($specificquestionids, $latestquestionids);
$questiondata = [];
if (!empty($questionids)) {
$questiondata = \mod_quiz\question\bank\qbank_helper::get_question_structure_data($this->quiz->id, $questionids, true);
$slots = qbank_helper::get_question_structure($this->quiz->id, $this->context);
$this->questions = [];
foreach ($slots as $slot) {
$this->questions[$slot->questionid] = $slot;
}
$allquestiondata = \mod_quiz\question\bank\qbank_helper::question_load_random_questions(
$this->quiz->id, $questiondata);
$this->questions = $allquestiondata;
}

/**
* Fully load some or all of the questions for this quiz. You must call
* {@link preload_questions()} first.
*
* @param array|null $questionids question ids of the questions to load. null for all.
* @param array|null $deprecated no longer supported (it was not used).
*/
public function load_questions($questionids = null) {
public function load_questions($deprecated = null) {
if ($deprecated !== null) {
debugging('The argument to quiz::load_questions is no longer supported. ' .
'All questions are always loaded.', DEBUG_DEVELOPER);
}
if ($this->questions === null) {
throw new coding_exception('You must call preload_questions before calling load_questions.');
}
if (is_null($questionids)) {
$questionids = array_keys($this->questions);
}
$questionstoprocess = array();
foreach ($questionids as $id) {
if (array_key_exists($id, $this->questions)) {
$questionstoprocess[$id] = $this->questions[$id];

$questionstoprocess = [];
foreach ($this->questions as $question) {
if (is_number($question->questionid)) {
$question->id = $question->questionid;
$questionstoprocess[$question->questionid] = $question;
}
}
get_question_options($questionstoprocess);
Expand Down Expand Up @@ -540,8 +543,17 @@ public function get_all_question_types_used($includepotential = false) {
$qcategories = array();

foreach ($this->get_questions() as $questiondata) {
if (!in_array($questiondata->qtype, $questiontypes)) {
$questiontypes[] = $questiondata->qtype;
if ($questiondata->qtype == 'random' and $includepotential) {
if (!isset($qcategories[$questiondata->category])) {
$qcategories[$questiondata->category] = false;
}
if ($questiondata->includingsubcategories) {
$qcategories[$questiondata->category] = true;
}
} else {
if (!in_array($questiondata->qtype, $questiontypes)) {
$questiontypes[] = $questiondata->qtype;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion mod/quiz/backup/moodle2/restore_quiz_stepslib.php
View file
Expand Up @@ -353,7 +353,7 @@ protected function process_quiz_question_legacy_instance($data) {
$questionreference->questionarea = 'slot';
$questionreference->itemid = $data->id;
$questionreference->questionbankentryid = $question->questionbankentryid;
$questionreference->version = $question->version;
$questionreference->version = null; // Default to Always latest.
$DB->insert_record('question_references', $questionreference);
}
}
Expand Down
38 changes: 21 additions & 17 deletions mod/quiz/classes/output/edit_renderer.php
View file
Expand Up @@ -756,7 +756,7 @@ public function question(structure $structure, int $slot, \moodle_url $pageurl)
$data['versionoptions'] = [];
if ($structure->get_slot_by_number($slot)->qtype !== 'random') {
$data['versionselection'] = true;
$data['versionoption'] = qbank_helper::get_question_version_info($structure->get_question_in_slot($slot)->id, $slotid);
$data['versionoption'] = $structure->get_version_choices_for_slot($slot);
$this->page->requires->js_call_amd('mod_quiz/question_slot', 'init', [$slotid]);
}

Expand Down Expand Up @@ -819,8 +819,9 @@ public function get_action_icon(structure $structure, int $slot, \moodle_url $pa
$qtype = $structure->get_question_type_for_slot($slot);
$questionicons = '';
if ($qtype !== 'random') {
$questionicons .= $this->question_preview_icon($structure->get_quiz(), $structure->get_question_in_slot($slot),
null, null, $qtype);
$questionicons .= $this->question_preview_icon($structure->get_quiz(),
$structure->get_question_in_slot($slot),
null, null, $qtype);
}
if ($structure->can_be_edited()) {
$questionicons .= $this->question_remove_icon($structure, $slot, $pageurl);
Expand Down Expand Up @@ -860,13 +861,19 @@ public function question_number($number) {
* Render the preview icon.
*
* @param \stdClass $quiz the quiz settings from the database.
* @param \stdClass $question data from the question and quiz_slots tables.
* @param \stdClass $questiondata which question to preview.
* If ->questionid is set, that is used instead of ->id.
* @param bool $label if true, show the preview question label after the icon
* @param int $variant which question variant to preview (optional).
* @param string $qtype the type of question
* @return string HTML to output.
*/
public function question_preview_icon($quiz, $question, $label = null, $variant = null, $qtype = null) {
public function question_preview_icon($quiz, $questiondata, $label = null, $variant = null) {
$question = clone($questiondata);
if (isset($question->questionid)) {

$question->id = $question->questionid;
}

$url = quiz_question_preview_url($quiz, $question, $variant);

// Do we want a label?
Expand Down Expand Up @@ -992,7 +999,7 @@ public function question_name(structure $structure, $slot, $pageurl) {
$question = $structure->get_question_in_slot($slot);
$editurl = new \moodle_url('/question/bank/editquestion/question.php', array(
'returnurl' => $pageurl->out_as_local_url(),
'cmid' => $structure->get_cmid(), 'id' => $question->id));
'cmid' => $structure->get_cmid(), 'id' => $question->questionid));

$instancename = quiz_question_tostring($question);

Expand Down Expand Up @@ -1034,25 +1041,22 @@ public function random_question(structure $structure, $slotnumber, $pageurl) {
$temp->questiontext = '';
$instancename = quiz_question_tostring($temp);

$setreference = qbank_helper::get_random_question_data_from_slot($slot->id);
$filtercondition = json_decode($setreference->filtercondition);

$configuretitle = get_string('configurerandomquestion', 'quiz');
$qtype = \question_bank::get_qtype($question->qtype, false);
$namestr = $qtype->local_name();
$icon = $this->pix_icon('icon', $namestr, $qtype->plugin_name(), array('title' => $namestr,
'class' => 'icon activityicon', 'alt' => ' ', 'role' => 'presentation'));

$editicon = $this->pix_icon('t/edit', $configuretitle, 'moodle', array('title' => ''));
$qbankurlparams = array(
'cmid' => $structure->get_cmid(),
'cat' => $filtercondition->questioncategoryid . ',' . $setreference->questionscontextid,
'recurse' => !empty($setreference->questionscontextid)
);
$qbankurlparams = [
'cmid' => $structure->get_cmid(),
'cat' => $slot->category . ',' . $slot->contextid,
'recurse' => $slot->randomrecurse,
];

$slottags = [];
if (isset($filtercondition->tags)) {
$slottags = $filtercondition->tags;
if (isset($slot->randomtags)) {
$slottags = $slot->randomtags;
}
foreach ($slottags as $index => $slottag) {
$slottag = explode(',', $slottag);
Expand Down

0 comments on commit 839ccce

Please sign in to comment.