From 3c724fecad1ea5404bd567a1b52f4d74d2b13b4f Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 8 Mar 2024 17:20:26 +0000 Subject: [PATCH] MDL-81114 question: fix selection of random questions with draft status We need to pick the latest 'ready' version of each question (not questions where the latest version is ready). There are test of the behaviour of random_question_loader, and then a test in the quiz to prove it works in use. --- mod/quiz/tests/attempt_test.php | 48 ++++++++++++++++- .../tests/quiz_question_helper_test_trait.php | 4 +- .../local/bank/random_question_loader.php | 13 +++-- .../tests/random_question_loader_test.php | 52 +++++++++++++++++++ 4 files changed, 107 insertions(+), 10 deletions(-) diff --git a/mod/quiz/tests/attempt_test.php b/mod/quiz/tests/attempt_test.php index 154b288f91f..1197179433b 100644 --- a/mod/quiz/tests/attempt_test.php +++ b/mod/quiz/tests/attempt_test.php @@ -16,8 +16,10 @@ namespace mod_quiz; +use core_question\local\bank\condition; use core_question\local\bank\question_version_status; -use mod_quiz\output\view_page; +use core_question_generator; +use mod_quiz_generator; use question_engine; defined('MOODLE_INTERNAL') || die(); @@ -32,6 +34,7 @@ require_once($CFG->dirroot . '/mod/quiz/locallib.php'); * @category test * @copyright 2014 Tim Hunt * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \mod_quiz\quiz_attempt */ class attempt_test extends \advanced_testcase { @@ -378,6 +381,49 @@ class attempt_test extends \advanced_testcase { $this->assertEquals($student1->id, $step->get_user_id()); } + /** + * Test quiz_prepare_and_start_new_attempt function + */ + public function test_quiz_prepare_and_start_new_attempt_random_draft(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create course. + $course = $this->getDataGenerator()->create_course(); + // Create quiz. + /** @var mod_quiz_generator $quizgenerator */ + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id]); + + // Create question with 2 versions. V1 ready. V2 draft. + /** @var core_question_generator $questiongenerator */ + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $question = $questiongenerator->create_question('shortanswer', null, + ['questiontext' => 'V1', 'category' => $category->id]); + $questiongenerator->update_question($question, null, + ['questiontext' => 'V2', 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + + // Add a random question form that category. + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'jointype' => condition::JOINTYPE_DEFAULT, + 'values' => [$category->id], + 'filteroptions' => ['includesubcategories' => false], + ], + ], + ]; + $quizobj = quiz_settings::create($quiz->id); + $quizobj->get_structure()->add_random_questions(1, 1, $filtercondition); + $quizobj->get_grade_calculator()->recompute_quiz_sumgrades(); + + // Create an attempt. + $quizobj = quiz_settings::create($quiz->id); + $attempt = quiz_prepare_and_start_new_attempt($quizobj, 1, null); + $this->assertEquals(1, $attempt->preview); + } + /** * Test check_page_access function * @covers \quiz_attempt::check_page_access diff --git a/mod/quiz/tests/quiz_question_helper_test_trait.php b/mod/quiz/tests/quiz_question_helper_test_trait.php index 198c41e9968..c5b2220c489 100644 --- a/mod/quiz/tests/quiz_question_helper_test_trait.php +++ b/mod/quiz/tests/quiz_question_helper_test_trait.php @@ -191,8 +191,8 @@ trait quiz_question_helper_test_trait { * @return void */ protected function add_random_questions(int $quizid, int $page, int $categoryid, int $number): void { - $settings = quiz_settings::create($quizid); - $structure = \mod_quiz\structure::create_for_quiz($settings); + $quizobj = quiz_settings::create($quizid); + $structure = $quizobj->get_structure(); $filtercondition = [ 'filter' => [ 'category' => [ diff --git a/question/classes/local/bank/random_question_loader.php b/question/classes/local/bank/random_question_loader.php index 3e75c5b0deb..8d66d2adca6 100644 --- a/question/classes/local/bank/random_question_loader.php +++ b/question/classes/local/bank/random_question_loader.php @@ -232,15 +232,14 @@ class random_question_loader { JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid WHERE q.parent = :noparent - AND qv.status = :ready - AND qv.version = ( - SELECT MAX(v.version) - FROM {question_versions} v - JOIN {question_bank_entries} be ON be.id = v.questionbankentryid - WHERE be.id = qbe.id - ) $qtypecondition $filtercondition + AND qv.version = ( + SELECT MAX(version) + FROM {question_versions} + WHERE questionbankentryid = qbe.id + AND status = :ready + ) ORDER BY previous_attempts ", array_merge( diff --git a/question/tests/random_question_loader_test.php b/question/tests/random_question_loader_test.php index 1931b5a3892..a977bbbae9d 100644 --- a/question/tests/random_question_loader_test.php +++ b/question/tests/random_question_loader_test.php @@ -120,6 +120,58 @@ final class random_question_loader_test extends \advanced_testcase { $this->assertNull($loader->get_next_filtered_question_id($filters)); } + public function test_draft_questions_not_returned(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + /** @var core_question_generator $questiongenerator */ + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question in draft state. + $category = $questiongenerator->create_question_category(); + $questiongenerator->create_question('shortanswer', null, + ['category' => $category->id, 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + + // Try to a random question from that category - should not be one. + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'jointype' => condition::JOINTYPE_DEFAULT, + 'values' => [$category->id], + 'filteroptions' => ['includesubcategories' => false], + ], + ], + ]; + $loader = new random_question_loader(new qubaid_list([])); + $this->assertNull($loader->get_next_filtered_question_id($filtercondition)); + } + + public function test_questions_with_later_draft_version_is_returned(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + /** @var core_question_generator $questiongenerator */ + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question in draft state. + $category = $questiongenerator->create_question_category(); + $question = $questiongenerator->create_question('shortanswer', null, + ['questiontext' => 'V1', 'category' => $category->id]); + $questiongenerator->update_question($question, null, + ['questiontext' => 'V2', 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + + // Try to a random question from that category - should get V1. + $filtercondition = [ + 'filter' => [ + 'category' => [ + 'jointype' => condition::JOINTYPE_DEFAULT, + 'values' => [$category->id], + 'filteroptions' => ['includesubcategories' => false], + ], + ], + ]; + $loader = new random_question_loader(new qubaid_list([])); + $this->assertEquals($question->id, $loader->get_next_filtered_question_id($filtercondition)); + } + public function test_one_question_category_returns_that_q_then_null(): void { $this->resetAfterTest(); $generator = $this->getDataGenerator()->get_plugin_generator('core_question');