From 4fa49cc647071826ccda00266cdcac9f52d07579 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Mon, 12 Feb 2018 02:03:45 +1100 Subject: [PATCH] MDL-61380 Quiz: Adding a random question --- lib/tests/questionlib_test.php | 20 ++++-- mod/quiz/locallib.php | 84 ++++++++++++++----------- mod/quiz/tests/behat/behat_mod_quiz.php | 20 ++++-- mod/quiz/tests/external_test.php | 10 +-- mod/quiz/upgrade.txt | 1 + question/type/random/tests/helper.php | 2 +- 6 files changed, 86 insertions(+), 51 deletions(-) diff --git a/lib/tests/questionlib_test.php b/lib/tests/questionlib_test.php index da09a5b2d91..bc5874630ea 100644 --- a/lib/tests/questionlib_test.php +++ b/lib/tests/questionlib_test.php @@ -398,6 +398,8 @@ class core_questionlib_testcase extends advanced_testcase { public function test_question_remove_stale_questions_from_category() { global $DB; $this->resetAfterTest(true); + $this->setAdminUser(); + $dg = $this->getDataGenerator(); $course = $dg->create_course(); $quiz = $dg->create_module('quiz', ['course' => $course->id]); @@ -407,18 +409,28 @@ class core_questionlib_testcase extends advanced_testcase { $qcat1 = $qgen->create_question_category(['contextid' => $context->id]); $q1a = $qgen->create_question('shortanswer', null, ['category' => $qcat1->id]); // Will be hidden. - $q1b = $qgen->create_question('random', null, ['category' => $qcat1->id]); // Will not be used. $DB->set_field('question', 'hidden', 1, ['id' => $q1a->id]); $qcat2 = $qgen->create_question_category(['contextid' => $context->id]); $q2a = $qgen->create_question('shortanswer', null, ['category' => $qcat2->id]); // Will be hidden. $q2b = $qgen->create_question('shortanswer', null, ['category' => $qcat2->id]); // Will be hidden but used. - $q2c = $qgen->create_question('random', null, ['category' => $qcat2->id]); // Will not be used. - $q2d = $qgen->create_question('random', null, ['category' => $qcat2->id]); // Will be used. $DB->set_field('question', 'hidden', 1, ['id' => $q2a->id]); $DB->set_field('question', 'hidden', 1, ['id' => $q2b->id]); quiz_add_quiz_question($q2b->id, $quiz); - quiz_add_quiz_question($q2d->id, $quiz); + quiz_add_random_questions($quiz, 0, $qcat2->id, 1, false); + + // We added one random question to the quiz and we expect the quiz to have only one random question. + $q2d = $DB->get_record_sql("SELECT q.* + FROM {question} q + JOIN {quiz_slots} s ON s.questionid = q.id + WHERE q.qtype = :qtype + AND s.quizid = :quizid", + array('qtype' => 'random', 'quizid' => $quiz->id), MUST_EXIST); + + // The following 2 lines have to be after the quiz_add_random_questions() call above. + // Otherwise, quiz_add_random_questions() will to be "smart" and use them instead of creating a new "random" question. + $q1b = $qgen->create_question('random', null, ['category' => $qcat1->id]); // Will not be used. + $q2c = $qgen->create_question('random', null, ['category' => $qcat2->id]); // Will not be used. $this->assertEquals(2, $DB->count_records('question', ['category' => $qcat1->id])); $this->assertEquals(4, $DB->count_records('question', ['category' => $qcat2->id])); diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 19d6bc2641f..0561694388b 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -2072,6 +2072,15 @@ function quiz_has_question_use($quiz, $slot) { */ function quiz_add_quiz_question($questionid, $quiz, $page = 0, $maxmark = null) { global $DB; + + // Make sue the question is not of the "random" type. + $questiontype = $DB->get_field('question', 'qtype', array('id' => $questionid)); + if ($questiontype == 'random') { + throw new coding_exception( + 'Adding "random" questions via quiz_add_quiz_question() is deprecated. Please use quiz_add_random_questions().' + ); + } + $slots = $DB->get_records('quiz_slots', array('quizid' => $quiz->id), 'slot', 'questionid, slot, page, id'); if (array_key_exists($questionid, $slots)) { @@ -2159,7 +2168,7 @@ function quiz_update_section_firstslots($quizid, $direction, $afterslot, $before /** * Add a random question to the quiz at a given point. - * @param object $quiz the quiz settings. + * @param stdClass $quiz the quiz settings. * @param int $addonpage the page on which to add the question. * @param int $categoryid the question category to add the question from. * @param int $number the number of random questions to add. @@ -2177,44 +2186,49 @@ function quiz_add_random_questions($quiz, $addonpage, $categoryid, $number, $catcontext = context::instance_by_id($category->contextid); require_capability('moodle/question:useall', $catcontext); + $tags = []; + // Find existing random questions in this category that are // not used by any quiz. - if ($existingquestions = $DB->get_records_sql( - "SELECT q.id, q.qtype FROM {question} q - WHERE qtype = 'random' - AND category = ? - AND " . $DB->sql_compare_text('questiontext') . " = ? - AND NOT EXISTS ( - SELECT * - FROM {quiz_slots} - WHERE questionid = q.id) - ORDER BY id", array($category->id, ($includesubcategories ? '1' : '0')))) { - // Take as many of these as needed. - while (($existingquestion = array_shift($existingquestions)) && $number > 0) { - quiz_add_quiz_question($existingquestion->id, $quiz, $addonpage); - $number -= 1; - } - } + $existingquestions = $DB->get_records_sql( + "SELECT q.id, q.qtype FROM {question} q + WHERE qtype = 'random' + AND category = ? + AND " . $DB->sql_compare_text('questiontext') . " = ? + AND NOT EXISTS ( + SELECT * + FROM {quiz_slots} + WHERE questionid = q.id) + ORDER BY id", array($category->id, ($includesubcategories ? '1' : '0'))); - if ($number <= 0) { - return; - } - - // More random questions are needed, create them. - for ($i = 0; $i < $number; $i += 1) { - $form = new stdClass(); - $form->questiontext = array('text' => ($includesubcategories ? '1' : '0'), 'format' => 0); - $form->category = $category->id . ',' . $category->contextid; - $form->defaultmark = 1; - $form->hidden = 1; - $form->stamp = make_unique_id_code(); // Set the unique code (not to be changed). - $question = new stdClass(); - $question->qtype = 'random'; - $question = question_bank::get_qtype('random')->save_question($question, $form); - if (!isset($question->id)) { - print_error('cannotinsertrandomquestion', 'quiz'); + for ($i = 0; $i < $number; $i++) { + // Take as many of orphaned "random" questions as needed. + if (!$question = array_shift($existingquestions)) { + $form = new stdClass(); + $form->questiontext = array('text' => ($includesubcategories ? '1' : '0'), 'format' => 0); + $form->category = $category->id . ',' . $category->contextid; + $form->defaultmark = 1; + $form->hidden = 1; + $form->stamp = make_unique_id_code(); // Set the unique code (not to be changed). + $question = new stdClass(); + $question->qtype = 'random'; + $question = question_bank::get_qtype('random')->save_question($question, $form); + if (!isset($question->id)) { + print_error('cannotinsertrandomquestion', 'quiz'); + } } - quiz_add_quiz_question($question->id, $quiz, $addonpage); + + $randomslotdata = new stdClass(); + $randomslotdata->quizid = $quiz->id; + $randomslotdata->questionid = $question->id; + $randomslotdata->questioncategoryid = $categoryid; + $randomslotdata->includingsubcategories = $includesubcategories ? 1 : 0; + $randomslotdata->tags = json_encode($tags); + $randomslotdata->maxmark = 1; + + $randomslot = new \mod_quiz\local\structure\slot_random($randomslotdata); + $randomslot->set_quiz($quiz); + $randomslot->insert($addonpage); } } diff --git a/mod/quiz/tests/behat/behat_mod_quiz.php b/mod/quiz/tests/behat/behat_mod_quiz.php index c18fea9e7fb..4054f62c5cf 100644 --- a/mod/quiz/tests/behat/behat_mod_quiz.php +++ b/mod/quiz/tests/behat/behat_mod_quiz.php @@ -78,7 +78,7 @@ class behat_mod_quiz extends behat_question_base { $headings = array('question', 'page', 'maxmark'); } else { throw new ExpectationException('When adding questions to a quiz, you should give 2 or three 3 things: ' . - ' the question name, the page number, and optionally the maxiumum mark. ' . + ' the question name, the page number, and optionally the maximum mark. ' . count($firstrow) . ' values passed.', $this->getSession()); } $rows = $data->getRows(); @@ -98,9 +98,8 @@ class behat_mod_quiz extends behat_question_base { 'the page number column is required.', $this->getSession()); } - // Question id. - $questionid = $DB->get_field('question', 'id', - array('name' => $questiondata['question']), MUST_EXIST); + // Question id, category and type. + $question = $DB->get_record('question', array('name' => $questiondata['question']), 'id, category, qtype', MUST_EXIST); // Page number. $page = clean_param($questiondata['page'], PARAM_INT); @@ -129,8 +128,17 @@ class behat_mod_quiz extends behat_question_base { } } - // Add the question. - quiz_add_quiz_question($questionid, $quiz, $page, $maxmark); + if ($question->qtype == 'random') { + if (!array_key_exists('includingsubcategories', $questiondata) || $questiondata['includingsubcategories'] === '') { + $includingsubcategories = false; + } else { + $includingsubcategories = clean_param($questiondata['includingsubcategories'], PARAM_BOOL); + } + quiz_add_random_questions($quiz, $page, $question->category, 1, $includingsubcategories); + } else { + // Add the question. + quiz_add_quiz_question($question->id, $quiz, $page, $maxmark); + } // Require previous. if (array_key_exists('requireprevious', $questiondata)) { diff --git a/mod/quiz/tests/external_test.php b/mod/quiz/tests/external_test.php index 015ba422564..ed5cc3a17c1 100644 --- a/mod/quiz/tests/external_test.php +++ b/mod/quiz/tests/external_test.php @@ -1587,6 +1587,8 @@ class mod_quiz_external_testcase extends externallib_advanced_testcase { public function test_get_attempt_access_information() { global $DB; + $this->setAdminUser(); + // Create a new quiz with attempts. $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); $data = array('course' => $this->course->id, @@ -1607,8 +1609,7 @@ class mod_quiz_external_testcase extends externallib_advanced_testcase { $question = $questiongenerator->create_question('truefalse', null, array('category' => $cat->id)); $question = $questiongenerator->create_question('essay', null, array('category' => $cat->id)); - $question = $questiongenerator->create_question('random', null, array('category' => $cat->id)); - quiz_add_quiz_question($question->id, $quiz); + quiz_add_random_questions($quiz, 0, $cat->id, 1, false); $quizobj = quiz::create($quiz->id, $this->student->id); @@ -1670,7 +1671,7 @@ class mod_quiz_external_testcase extends externallib_advanced_testcase { * Test get_quiz_required_qtypes */ public function test_get_quiz_required_qtypes() { - global $DB; + $this->setAdminUser(); // Create a new quiz. $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); @@ -1691,8 +1692,7 @@ class mod_quiz_external_testcase extends externallib_advanced_testcase { $question = $questiongenerator->create_question('truefalse', null, array('category' => $cat->id)); $question = $questiongenerator->create_question('essay', null, array('category' => $cat->id)); - $question = $questiongenerator->create_question('random', null, array('category' => $cat->id)); - quiz_add_quiz_question($question->id, $quiz); + quiz_add_random_questions($quiz, 0, $cat->id, 1, false); $this->setUser($this->student); diff --git a/mod/quiz/upgrade.txt b/mod/quiz/upgrade.txt index 4f3239fc62d..2319c92db6f 100644 --- a/mod/quiz/upgrade.txt +++ b/mod/quiz/upgrade.txt @@ -2,6 +2,7 @@ This files describes API changes in the quiz code. === 3.5 === * Removed questionbank.ajax.php. Please use the quiz_question_bank fragment instead. +* Adding "random" questions to a quiz via quiz_add_quiz_question() has been deprecated. Please use quiz_add_random_questions(). === 3.3.2 === diff --git a/question/type/random/tests/helper.php b/question/type/random/tests/helper.php index f857b603340..3fabfaac50f 100644 --- a/question/type/random/tests/helper.php +++ b/question/type/random/tests/helper.php @@ -46,7 +46,7 @@ class qtype_random_test_helper extends question_test_helper { public function get_random_question_form_data_basic() { $form = new stdClass(); $form->questiontext = array('text' => ''); - $form->includingsubcategories = '0'; + $form->includesubcategories = '0'; return $form; } }