diff --git a/.upgradenotes/MDL-77713-2024120511165062.yml b/.upgradenotes/MDL-77713-2024120511165062.yml new file mode 100644 index 00000000000..fc5d144b621 --- /dev/null +++ b/.upgradenotes/MDL-77713-2024120511165062.yml @@ -0,0 +1,10 @@ +issueNumber: MDL-77713 +notes: + core_question: + - message: >- + Deprecated method + `mod_quiz\question\bank\qbank_helper::get_version_options` in favour of + `core_question\local\bank\version_options::get_version_options` so that + the method is in core rather than a module, and can safely be used from + anywhere as required. + type: deprecated diff --git a/lang/en/question.php b/lang/en/question.php index 961095d64bf..4332da09cf2 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -27,6 +27,7 @@ $string['addcategory'] = 'Add category'; $string['adminreport'] = 'Report on possible problems in your question database.'; $string['advancedsearchoptions'] = 'Search options'; $string['alltries'] = 'All tries'; +$string['alwayslatest'] = 'Always latest'; $string['answers'] = 'Answers'; $string['availableq'] = 'Available?'; $string['badbase'] = 'Bad base before **: {$a}**'; @@ -533,6 +534,7 @@ $string['version_selection'] = 'Version {$a->version}'; $string['versioninfo'] = 'Version {$a->version} (of {$a->latestversion})'; $string['versioninfolatest'] = 'Version {$a->version} (latest)'; $string['question_version'] = 'Question version'; - +$string['question_versionshort'] = 'v{$a}'; +$string['versioninfolatestshort'] = 'v{$a} (latest)'; // Deprecated since Moodle 5.0. $string['errordeletingquestionsfromcategory'] = 'Error deleting questions from category {$a}.'; diff --git a/mod/quiz/classes/question/bank/qbank_helper.php b/mod/quiz/classes/question/bank/qbank_helper.php index 2ee0b3a3102..553e03cd749 100644 --- a/mod/quiz/classes/question/bank/qbank_helper.php +++ b/mod/quiz/classes/question/bank/qbank_helper.php @@ -17,6 +17,7 @@ namespace mod_quiz\question\bank; use context_module; +use core_question\local\bank\version_options; use core_question\local\bank\question_version_status; use core_question\local\bank\random_question_loader; use core_question\question_reference_manager; @@ -46,25 +47,14 @@ class qbank_helper { * @return stdClass[] other versions of this question. Each object has fields versionid, * version and questionid. Array is returned most recent version first. */ + #[\core\attribute\deprecated( + 'core_question\local\bank::get_version_options', + since: 5.0, + mdl: 'MDL-77713') + ] public static function get_version_options(int $questionid): array { - global $DB; - - return $DB->get_records_sql(" - SELECT allversions.id AS versionid, - allversions.version, - allversions.questionid - - FROM {question_versions} allversions - - WHERE allversions.questionbankentryid = ( - SELECT givenversion.questionbankentryid - FROM {question_versions} givenversion - WHERE givenversion.questionid = ? - ) - AND allversions.status <> ? - - ORDER BY allversions.version DESC - ", [$questionid, question_version_status::QUESTION_STATUS_DRAFT]); + \core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]); + return version_options::get_version_options($questionid); } /** diff --git a/mod/quiz/classes/structure.php b/mod/quiz/classes/structure.php index cbbf803ae80..dc25743164e 100644 --- a/mod/quiz/classes/structure.php +++ b/mod/quiz/classes/structure.php @@ -19,6 +19,7 @@ namespace mod_quiz; use coding_exception; use context_module; use core\output\inplace_editable; +use core_question\local\bank\version_options; use mod_quiz\event\quiz_grade_item_created; use mod_quiz\event\quiz_grade_item_deleted; use mod_quiz\event\quiz_grade_item_updated; @@ -813,32 +814,19 @@ class structure { $slot = $this->get_slot_by_number($slotnumber); // Get all the versions which exist. - $versions = qbank_helper::get_version_options($slot->questionid); - $latestversion = reset($versions); + $versions = version_options::get_version_menu_options($slot->questionid); + $versioninfo = []; - // Format the choices for display. - $versionoptions = []; - foreach ($versions as $version) { - $version->selected = $version->version === $slot->requestedversion; - - if ($version->version === $latestversion->version) { - $version->versionvalue = get_string('questionversionlatest', 'quiz', $version->version); - } else { - $version->versionvalue = get_string('questionversion', 'quiz', $version->version); - } - - $versionoptions[] = $version; + // Loop through them and set which one is selected. + foreach ($versions as $versionnumber => $version) { + $versioninfo[] = (object)[ + 'version' => $versionnumber, + 'versionvalue' => $version, + 'selected' => ($versionnumber == $slot->requestedversion), + ]; } - // Make a choice for 'Always latest'. - $alwaysuselatest = new stdClass(); - $alwaysuselatest->versionid = 0; - $alwaysuselatest->version = 0; - $alwaysuselatest->versionvalue = get_string('alwayslatest', 'quiz'); - $alwaysuselatest->selected = $slot->requestedversion === null; - array_unshift($versionoptions, $alwaysuselatest); - - return $versionoptions; + return $versioninfo; } /** diff --git a/mod/quiz/tests/qbank_helper_test.php b/mod/quiz/tests/qbank_helper_test.php index 56c28c57bf4..7f9cfe22fb4 100644 --- a/mod/quiz/tests/qbank_helper_test.php +++ b/mod/quiz/tests/qbank_helper_test.php @@ -88,6 +88,7 @@ final class qbank_helper_test extends \advanced_testcase { $slots = $structure->get_slots(); $slot = reset($slots); $this->assertEquals(3, count(qbank_helper::get_version_options($question->id))); + $this->assertDebuggingCalled(); $this->assertEquals($question->id, qbank_helper::choose_question_for_redo( $quiz->id, $context, $slot->id, new \qubaid_list([]))); diff --git a/mod/quiz/tests/quiz_question_version_test.php b/mod/quiz/tests/quiz_question_version_test.php index b0542e81be8..8a65d47d652 100644 --- a/mod/quiz/tests/quiz_question_version_test.php +++ b/mod/quiz/tests/quiz_question_version_test.php @@ -97,6 +97,7 @@ final class quiz_question_version_test extends \advanced_testcase { $this->assertEquals(4, $slot->version); // Now change the version using the external service. $versions = qbank_helper::get_version_options($slot->questionid); + $this->assertDebuggingCalled(); // We don't want the current version. $selectversions = []; foreach ($versions as $version) { @@ -157,6 +158,7 @@ final class quiz_question_version_test extends \advanced_testcase { $slot = reset($slots); // Now change the version using the external service. $versions = qbank_helper::get_version_options($slot->questionid); + $this->assertDebuggingCalled(); // We dont want the current version. $selectversions = []; foreach ($versions as $version) { diff --git a/question/bank/previewquestion/classes/form/preview_options_form.php b/question/bank/previewquestion/classes/form/preview_options_form.php index 281df81877a..5aa7bebddad 100644 --- a/question/bank/previewquestion/classes/form/preview_options_form.php +++ b/question/bank/previewquestion/classes/form/preview_options_form.php @@ -49,7 +49,6 @@ class preview_options_form extends moodleform { $mform->addElement('html', \html_writer::div(get_string('theoptionsyouselectonlyaffectthepreview', 'qbank_previewquestion'), "col-md-12 row d-flex col-form-label mb-3")); $versions = $this->_customdata['versions']; - $versions[question_preview_options::ALWAYS_LATEST] = get_string('alwayslatest', 'qbank_previewquestion'); $currentversion = $this->_customdata['restartversion']; $select = $mform->addElement('select', 'restartversion', get_string('questionversion', 'qbank_previewquestion'), $versions); $select->setSelected($currentversion); diff --git a/question/bank/previewquestion/preview.php b/question/bank/previewquestion/preview.php index a7a9e10d8d5..cb20f655126 100644 --- a/question/bank/previewquestion/preview.php +++ b/question/bank/previewquestion/preview.php @@ -35,6 +35,7 @@ use \core\notification; use qbank_previewquestion\form\preview_options_form; use qbank_previewquestion\question_preview_options; use qbank_previewquestion\helper; +use core_question\local\bank\version_options; /** * The maximum number of variants previewable. If there are more variants than this for a question @@ -130,13 +131,15 @@ if ($previewid) { $options->behaviour = $quba->get_preferred_behaviour(); $options->maxmark = $quba->get_question_max_mark($slot); +// Load the question versions and convert to a simple array for select menu. +$versions = version_options::get_version_menu_options($question->id); $versionids = helper::load_versions($question->questionbankentryid); // Create the settings form, and initialise the fields. $optionsform = new preview_options_form(helper::question_preview_form_url($question->id, $context, $previewid, $returnurl), [ 'quba' => $quba, 'maxvariant' => $maxvariant, - 'versions' => array_combine(array_values($versionids), array_values($versionids)), + 'versions' => $versions, 'restartversion' => $restartversion, ]); $optionsform->set_data($options); diff --git a/question/bank/previewquestion/tests/behat/preview_question.feature b/question/bank/previewquestion/tests/behat/preview_question.feature index 54bbad8da42..d1f2cdd5c57 100644 --- a/question/bank/previewquestion/tests/behat/preview_question.feature +++ b/question/bank/previewquestion/tests/behat/preview_question.feature @@ -101,19 +101,17 @@ Feature: A teacher can preview questions in the question bank And I click on "submitbutton" "button" And I choose "Preview" action for "New version" in the question bank When I expand all fieldsets - And I should see "Version 2" - And I should see "(latest)" + And I should see "Version 2 (latest)" And I should see "New version" And I should see "New text version" And I should not see "Test question to be previewed" And I should not see "Version 1" - And I should see "1" in the "Question version" "select" - And I should see "2" in the "Question version" "select" - And I set the field "Question version" to "1" + And I should see "v1" in the "Question version" "select" + And I should see "v2" in the "Question version" "select" + And I set the field "Question version" to "v1" And I press "Save preview options and start again" Then I should see "Version 1" - And I should not see "Version 2" - And I should not see "(latest)" + And I should not see "Version 2 (latest)" Scenario: The preview always uses the latest question version by default. Given the following "core_question > updated questions" exist: diff --git a/question/classes/local/bank/version_options.php b/question/classes/local/bank/version_options.php new file mode 100644 index 00000000000..67ca280c631 --- /dev/null +++ b/question/classes/local/bank/version_options.php @@ -0,0 +1,86 @@ +. + +namespace core_question\local\bank; + +/** + * Helper methods for question version options. + * + * @package core_question + * @copyright 2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @author Conn Warwicker + */ +class version_options { + + /** + * Get the version options for display in dropdown menu + * @param int $questionid + * @return array + * @throws \coding_exception + */ + public static function get_version_menu_options(int $questionid): array { + + $versions = self::get_version_options($questionid); + $latestversion = reset($versions); + + $return = []; + + // Add the "always latest" option to the beginning. + $return[0] = get_string('alwayslatest', 'question'); + + foreach ($versions as $version) { + if ($version->version === $latestversion->version) { + $value = get_string('versioninfolatestshort', 'question', $version->version); + } else { + $value = get_string('question_versionshort', 'question', $version->version); + } + $return[$version->version] = $value; + } + + return $return; + + } + + /** + * Get the available versions of a question where one of the version has the given question id. + * + * @param int $questionid id of a question. + * @return stdClass[] other versions of this question. Each object has fields versionid, + * version and questionid. Array is returned most recent version first. + */ + public static function get_version_options(int $questionid): array { + global $DB; + + return $DB->get_records_sql(" + SELECT allversions.id AS versionid, + allversions.version, + allversions.questionid + + FROM {question_versions} allversions + + WHERE allversions.questionbankentryid = ( + SELECT givenversion.questionbankentryid + FROM {question_versions} givenversion + WHERE givenversion.questionid = ? + ) + AND allversions.status <> ? + + ORDER BY allversions.version DESC + ", [$questionid, question_version_status::QUESTION_STATUS_DRAFT]); + } + +} diff --git a/question/tests/local/bank/version_options_test.php b/question/tests/local/bank/version_options_test.php new file mode 100644 index 00000000000..51de458040c --- /dev/null +++ b/question/tests/local/bank/version_options_test.php @@ -0,0 +1,90 @@ +. + +namespace core_question\local\bank; + +/** + * Unit tests for the version_options class and associated methods. + * + * @package core_question + * @copyright 2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @author Conn Warwicker + * @covers \core_question\local\bank\version_options + */ +final class version_options_test extends \advanced_testcase { + + + /** + * Tests the retrieval and correctness of version selection menu options for a given question. + * + * This method verifies that the correct version options are available for a specific question + * within a course, ensuring that the options are accurately labeled and ordered. It includes + * checks for version labeling, the presence of an "Always latest" option, and the correct count + * and sequence of version numbers after creating multiple versions of a question. + * + * @return void + */ + public function test_get_version_options(): void { + + // Reset everything after this test completes. + $this->resetAfterTest(); + + // Load the generators we need. + $questiongenerator = self::getDataGenerator()->get_plugin_generator('core_question'); + + // Create a test course we can use the question bank on. + $category = self::getDataGenerator()->create_category(); + $course = self::getDataGenerator()->create_course(['category' => $category->id]); + $coursecontext = \core\context\course::instance($course->id); + + // Create a question category on the course. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create a question within that category. + $question = $questiongenerator->create_question('essay', null, + ['category' => $cat->id, 'name' => 'This is the first version']); + + // Get the select menu options for the question versions. + $options = version_options::get_version_menu_options($question->id); + + // Assert that there are 2 and that they "Always latest" is first. + $this->assertCount(2, $options); + $this->assertEquals('Always latest', $options[0]); + $this->assertEquals('v1 (latest)', $options[1]); + + // Create some new versions of the question. + $questiongenerator->update_question($question, null, ['name' => 'This is the second version']); + $questiongenerator->update_question($question, null, ['name' => 'This is the third version']); + + // Get the options which would be used for the select menu. + $options = version_options::get_version_menu_options($question->id); + + // Assert that there are now 4. + $this->assertCount(4, $options); + + // Assert that they are keyed to their verison numbers. + $this->assertEquals('Always latest', $options[0]); + $this->assertEquals('v1', $options[1]); + $this->assertEquals('v2', $options[2]); + $this->assertEquals('v3 (latest)', $options[3]); + + // Assert that they are in the correct order of "Always latest" first, then descending versions. + $this->assertEquals(array_keys($options), [0, 3, 2, 1]); + + } + +}