diff --git a/mod/quiz/attemptlib.php b/mod/quiz/attemptlib.php index c2b1aab7472..c48abd26b34 100644 --- a/mod/quiz/attemptlib.php +++ b/mod/quiz/attemptlib.php @@ -27,6 +27,7 @@ defined('MOODLE_INTERNAL') || die(); +use core_question\local\bank\question_version_status; use mod_quiz\question\bank\qbank_helper; @@ -543,6 +544,10 @@ class quiz { $qcategories = array(); foreach ($this->get_questions() as $questiondata) { + if ($questiondata->status == question_version_status::QUESTION_STATUS_DRAFT) { + // Skip questions where all versions are draft. + continue; + } if ($questiondata->qtype === 'random' && $includepotential) { if (!isset($qcategories[$questiondata->category])) { $qcategories[$questiondata->category] = false; diff --git a/mod/quiz/classes/output/edit_renderer.php b/mod/quiz/classes/output/edit_renderer.php index a38326ac827..1d1151e1f86 100644 --- a/mod/quiz/classes/output/edit_renderer.php +++ b/mod/quiz/classes/output/edit_renderer.php @@ -25,6 +25,7 @@ namespace mod_quiz\output; defined('MOODLE_INTERNAL') || die(); +use core_question\local\bank\question_version_status; use mod_quiz\question\bank\qbank_helper; use \mod_quiz\structure; use \html_writer; @@ -758,7 +759,8 @@ class edit_renderer extends \plugin_renderer_base { 'questionname' => $this->get_question_name_for_slot($structure, $slot, $pageurl), 'questionicons' => $this->get_action_icon($structure, $slot, $pageurl), 'questiondependencyicon' => ($structure->can_be_edited() ? $this->question_dependency_icon($structure, $slot) : ''), - 'versionselection' => false + 'versionselection' => false, + 'draftversion' => $structure->get_question_in_slot($slot)->status == question_version_status::QUESTION_STATUS_DRAFT, ]; $data['versionoptions'] = []; diff --git a/mod/quiz/classes/question/bank/qbank_helper.php b/mod/quiz/classes/question/bank/qbank_helper.php index 2d551d1dab7..5b991ce1c2c 100644 --- a/mod/quiz/classes/question/bank/qbank_helper.php +++ b/mod/quiz/classes/question/bank/qbank_helper.php @@ -134,7 +134,9 @@ class qbank_helper { -- just before the commit that added this comment. -- For relevant question_bank_entries, this gets the latest non-draft slot number. LEFT JOIN ( - SELECT lv.questionbankentryid, MAX(lv.version) AS version + SELECT lv.questionbankentryid, + MAX(CASE WHEN lv.status <> :draft THEN lv.version END) AS usableversion, + MAX(lv.version) AS anyversion FROM {quiz_slots} lslot JOIN {question_references} lqr ON lqr.usingcontextid = :quizcontextid2 AND lqr.component = 'mod_quiz' AND lqr.questionarea = 'slot' AND lqr.itemid = lslot.id @@ -142,13 +144,14 @@ class qbank_helper { WHERE lslot.quizid = :quizid2 $slotidtest2 AND lqr.version IS NULL - AND lv.status <> :draft GROUP BY lv.questionbankentryid ) latestversions ON latestversions.questionbankentryid = qr.questionbankentryid LEFT JOIN {question_versions} qv ON qv.questionbankentryid = qbe.id - -- Either specified version, or latest ready version. - AND qv.version = COALESCE(qr.version, latestversions.version) + -- Either specified version, or latest usable version, or a draft version. + AND qv.version = COALESCE(qr.version, + latestversions.usableversion, + latestversions.anyversion) LEFT JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid LEFT JOIN {question} q ON q.id = qv.questionid diff --git a/mod/quiz/lang/en/quiz.php b/mod/quiz/lang/en/quiz.php index 32dbb775c87..6958940c0a6 100644 --- a/mod/quiz/lang/en/quiz.php +++ b/mod/quiz/lang/en/quiz.php @@ -732,6 +732,9 @@ $string['questiondependencyadd'] = 'No restriction on when question {$a->thisq} $string['questiondependencyfree'] = 'No restriction on this question'; $string['questiondependencyremove'] = 'Question {$a->thisq} cannot be attempted until the previous question {$a->previousq} has been completed • Click to change'; $string['questiondependsonprevious'] = 'This question cannot be attempted until the previous question has been completed.'; +$string['questiondraftonly'] = 'The question \'{$a}\' has all versions in Draft status, so cannot be used. Visit the question bank and set the status to Ready.'; +$string['questiondraftwillnotwork'] = 'This question has all versions in Draft status. The quiz will not work with this question in place. Remove this question, +or visit the question bank and set the status to Ready.'; $string['questioninuse'] = 'The question \'{$a->questionname}\' is currently being used in:
{$a->quiznames}
The question will not be deleted from these quizzes but only from the category list.'; $string['questionmissing'] = 'Question for this session is missing'; $string['questionname'] = 'Question name'; diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 4c5b859e466..ad34459b297 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -179,6 +179,9 @@ function quiz_start_new_attempt($quizobj, $quba, $attempt, $attemptnumber, $time $slot += 1; $maxmark[$slot] = $questiondata->maxmark; $page[$slot] = $questiondata->page; + if ($questiondata->status == \core_question\local\bank\question_version_status::QUESTION_STATUS_DRAFT) { + throw new moodle_exception('questiondraftonly', 'mod_quiz', '', $questiondata->name); + } if ($questiondata->qtype == 'random') { $randomfound = true; continue; @@ -317,7 +320,11 @@ function quiz_start_attempt_built_on_last($quba, $attempt, $lastattempt) { $oldnumberstonew = array(); foreach ($oldquba->get_attempt_iterator() as $oldslot => $oldqa) { - $newslot = $quba->add_question($oldqa->get_question(false), $oldqa->get_max_mark()); + $question = $oldqa->get_question(false); + if ($question->status == \core_question\local\bank\question_version_status::QUESTION_STATUS_DRAFT) { + throw new moodle_exception('questiondraftonly', 'mod_quiz', '', $question->name); + } + $newslot = $quba->add_question($question, $oldqa->get_max_mark()); $quba->start_question_based_on($newslot, $oldqa); diff --git a/mod/quiz/templates/question_slot.mustache b/mod/quiz/templates/question_slot.mustache index c34a146b835..848d1ceb789 100644 --- a/mod/quiz/templates/question_slot.mustache +++ b/mod/quiz/templates/question_slot.mustache @@ -50,3 +50,6 @@ {{{questiondependencyicon}}} {{/canbeedited}} +{{#draftversion}} + +{{/draftversion}} diff --git a/mod/quiz/tests/attempt_test.php b/mod/quiz/tests/attempt_test.php index 0b78eae2cba..ee8884fb728 100644 --- a/mod/quiz/tests/attempt_test.php +++ b/mod/quiz/tests/attempt_test.php @@ -16,6 +16,7 @@ namespace mod_quiz; +use core_question\local\bank\question_version_status; use question_engine; use quiz; use quiz_attempt; @@ -431,4 +432,79 @@ class attempt_test extends \advanced_testcase { $this->assertFalse($attempt->check_page_access(4)); $this->assertFalse($attempt->check_page_access(1)); } + + /** + * Starting a new attempt with a question in draft status should throw an exception. + * + * @covers ::quiz_start_new_attempt() + * @return void + */ + public function test_start_new_attempt_with_draft(): void { + $this->resetAfterTest(); + + // Create course. + $course = $this->getDataGenerator()->create_course(); + // Create students. + $student1 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $student2 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + // Create quiz. + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id, 'grade' => 100.0, 'sumgrades' => 2, 'layout' => '1,0']); + // Create question and add it to quiz. + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $cat = $questiongenerator->create_question_category(); + $question = $questiongenerator->create_question('shortanswer', null, + ['category' => $cat->id, 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + quiz_add_quiz_question($question->id, $quiz, 1); + + $quizobj = quiz::create($quiz->id); + $quba = question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); + $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); + $attempt = quiz_create_attempt($quizobj, 1, false, time(), false, $student1->id); + + $this->expectExceptionObject(new \moodle_exception('questiondraftonly', 'mod_quiz', '', $question->name)); + quiz_start_new_attempt($quizobj, $quba, $attempt, 1, time()); + } + + /** + * Starting a new attempt built on last with a question in draft status should throw an exception. + * + * @covers ::quiz_start_attempt_built_on_last() + * @return void + */ + public function test_quiz_start_attempt_built_on_last_with_draft(): void { + global $DB; + $this->resetAfterTest(); + + // Create course. + $course = $this->getDataGenerator()->create_course(); + // Create students. + $student1 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $student2 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + // Create quiz. + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id, 'grade' => 100.0, 'sumgrades' => 2, 'layout' => '1,0']); + // Create question and add it to quiz. + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $cat = $questiongenerator->create_question_category(); + $question = $questiongenerator->create_question('shortanswer', null, ['category' => $cat->id]); + quiz_add_quiz_question($question->id, $quiz, 1); + + $quizobj = quiz::create($quiz->id); + $quba = question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); + $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); + $attempt = quiz_create_attempt($quizobj, 1, false, time(), false, $student1->id); + $attempt = quiz_start_new_attempt($quizobj, $quba, $attempt, 1, time()); + $attempt = quiz_attempt_save_started($quizobj, $quba, $attempt); + + $DB->set_field('question_versions', 'status', question_version_status::QUESTION_STATUS_DRAFT, + ['questionid' => $question->id]); + $quizobj = quiz::create($quiz->id); + $quba = question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); + $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); + $newattempt = quiz_create_attempt($quizobj, 2, $attempt, time(), false, $student1->id); + + $this->expectExceptionObject(new \moodle_exception('questiondraftonly', 'mod_quiz', '', $question->name)); + quiz_start_attempt_built_on_last($quba, $newattempt, $attempt); + } } diff --git a/mod/quiz/tests/behat/quiz_question_versions.feature b/mod/quiz/tests/behat/quiz_question_versions.feature index 96c84ad0c7c..356f5ec9473 100644 --- a/mod/quiz/tests/behat/quiz_question_versions.feature +++ b/mod/quiz/tests/behat/quiz_question_versions.feature @@ -111,3 +111,13 @@ Feature: Quiz question versioning And I press "Add selected questions to the quiz" Then I should see "Other question" on quiz page "1" And the field "version" in the "Other question" "list_item" matches value "Always latest" + + @javascript + Scenario: Adding a question where all available versions are drafts should display a helpful message. + Given quiz "Quiz 1" contains the following questions: + | question | page | + | First question | 1 | + And I am on the "Quiz 1" "mod_quiz > Question bank" page logged in as teacher + And I set the field "question_status_dropdown" in the "First question" "table_row" to "Draft" + When I am on the "Quiz 1" "mod_quiz > Edit" page + Then I should see "This question has all versions in Draft status. The quiz will not work with this question in place." diff --git a/mod/quiz/tests/external/external_test.php b/mod/quiz/tests/external/external_test.php index 30e4241d43a..e97b047aa01 100644 --- a/mod/quiz/tests/external/external_test.php +++ b/mod/quiz/tests/external/external_test.php @@ -26,6 +26,7 @@ namespace mod_quiz\external; +use core_question\local\bank\question_version_status; use externallib_advanced_testcase; use mod_quiz_external; use mod_quiz_display_options; @@ -1948,6 +1949,10 @@ class external_test extends externallib_advanced_testcase { $question = $questiongenerator->create_question('essay', null, array('category' => $cat->id)); quiz_add_quiz_question($question->id, $quiz); + $question = $questiongenerator->create_question('multichoice', null, + ['category' => $cat->id, 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + quiz_add_quiz_question($question->id, $quiz); + $this->setUser($this->student); $result = mod_quiz_external::get_quiz_required_qtypes($quiz->id); diff --git a/mod/quiz/tests/qbank_helper_test.php b/mod/quiz/tests/qbank_helper_test.php index 71d3474dbbf..d5784639efb 100644 --- a/mod/quiz/tests/qbank_helper_test.php +++ b/mod/quiz/tests/qbank_helper_test.php @@ -16,6 +16,7 @@ namespace mod_quiz; +use core_question\local\bank\question_version_status; use mod_quiz\external\submit_question_version; use mod_quiz\question\bank\qbank_helper; @@ -143,4 +144,63 @@ class qbank_helper_test extends \advanced_testcase { $slot = reset($slots); $this->assertEquals($finalq->id, $slot->questionid); } + + /** + * When a question only has draft versions, we should get those and not a dummy question. + * + * @return void + * @covers ::get_question_structure + */ + public function test_get_question_structure_with_drafts(): void { + $this->resetAfterTest(); + + // Create a quiz. + $quiz = $this->create_test_quiz($this->course); + $quizcontext = \context_module::instance(get_coursemodule_from_instance("quiz", $quiz->id, $this->course->id)->id); + + // Create some questions with drafts in the quiz question bank. + /** @var \core_question_generator $questiongenerator */ + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $cat = $questiongenerator->create_question_category(['contextid' => $quizcontext->id]); + $q1 = $questiongenerator->create_question('essay', null, + ['category' => $cat->id, 'name' => 'This is q1 the first version']); + $q2 = $questiongenerator->create_question('essay', null, + ['category' => $cat->id, 'name' => 'This is q2 the first version', + 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + $q3 = $questiongenerator->create_question('essay', null, + ['category' => $cat->id, 'name' => 'This is q3 the first version', + 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + + // Create a new draft version of a question. + $q1final = $questiongenerator->update_question(clone $q1, null, + ['name' => 'This is q1 the second version', 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + $q3final = $questiongenerator->update_question(clone $q3, null, + ['name' => 'This is q3 the second version', 'status' => question_version_status::QUESTION_STATUS_DRAFT]); + + // Add the questions to the quiz. + quiz_add_quiz_question($q1->id, $quiz); + quiz_add_quiz_question($q2->id, $quiz); + quiz_add_quiz_question($q3->id, $quiz); + + // Load the quiz object and check. + $quizobj = \quiz::create($quiz->id); + $quizobj->preload_questions(); + $quizobj->load_questions(); + $questions = $quizobj->get_questions(); + $this->assertCount(3, $questions); + // When a question has a Ready version, we should get that and not he draft. + $this->assertTrue(array_key_exists($q1->id, $questions)); + $this->assertFalse(array_key_exists($q1final->id, $questions)); + $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $questions[$q1->id]->status); + $this->assertEquals('essay', $questions[$q1->id]->qtype); + // When a question only has a draft, we should get that. + $this->assertTrue(array_key_exists($q2->id, $questions)); + $this->assertEquals(question_version_status::QUESTION_STATUS_DRAFT, $questions[$q2->id]->status); + $this->assertEquals('essay', $questions[$q2->id]->qtype); + // When a question has several versions but all draft, we should get the latest draft. + $this->assertFalse(array_key_exists($q3->id, $questions)); + $this->assertTrue(array_key_exists($q3final->id, $questions)); + $this->assertEquals(question_version_status::QUESTION_STATUS_DRAFT, $questions[$q3final->id]->status); + $this->assertEquals('essay', $questions[$q3final->id]->qtype); + } }