diff --git a/question/type/ordering/db/install.xml b/question/type/ordering/db/install.xml index 6183620dda0..fe4a49a9b0f 100644 --- a/question/type/ordering/db/install.xml +++ b/question/type/ordering/db/install.xml @@ -7,7 +7,7 @@ - + diff --git a/question/type/ordering/db/upgrade.php b/question/type/ordering/db/upgrade.php index e97624f3ec9..d93991cca23 100644 --- a/question/type/ordering/db/upgrade.php +++ b/question/type/ordering/db/upgrade.php @@ -326,7 +326,46 @@ function xmldb_qtype_ordering_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2023092911, 'qtype', 'ordering'); } + if ($oldversion < 2024040401) { + // The option to set "All" for the subset size ('selectcount') setting is no longer supported, therefore we need + // to update all data that references this option. + + // The 'selectcount' column from the 'qtype_ordering_options' table currently defines "0" as its default value. + // This value ("0") used to represent the removed "All" option for the 'selectcount' setting, therefore we need + // to update this to a new default of "2" which is the minimum number of items required to create a subset. + $table = new xmldb_table('qtype_ordering_options'); + $field = new xmldb_field('selectcount'); + $field->set_attributes(XMLDB_TYPE_INTEGER, '4', null, XMLDB_NOTNULL, null, 2); + $dbman->change_field_default($table, $field); + + // We need to find all ordering question configurations that currently store the unsupported "0" (all) option + // for the 'selectcount' setting and the total number of answers that are related to each of these ordering + // questions. + $sql = "SELECT qoo.*, COUNT(DISTINCT(qa.id)) AS answerscount + FROM {qtype_ordering_options} qoo + JOIN {question_answers} qa ON qa.question = qoo.questionid + WHERE selectcount = :selectcount + GROUP BY qoo.id"; + $questionoptions = $DB->get_recordset_sql($sql, ['selectcount' => 0]); + foreach($questionoptions as $questionoption) { + // Update the value of the 'selectcount' configuration option for the current ordering question and set it + // to the total number of answers related to this question. This way, we are making sure that the original + // behavior is preserved and all existing items (answers) related to the question will be included in the + // subset. + $questionoption->selectcount = $questionoption->answerscount; + unset($questionoption->answerscount); + $DB->update_record('qtype_ordering_options', $questionoption); + } + $questionoptions->close(); + + // Currently, a 'qtype_ordering_selectcount' user preference is set (or updated, if it already exists) each time + // a new ordering question is created. If there are user preferences that store the removed "0" (all) option, they + // need to be updated. In this case, replace it with "2" (minimum number of items required to create a subset). + $DB->set_field('user_preferences', 'value', 2, + ['name' => 'qtype_ordering_selectcount', 'value' => 0]); + + upgrade_plugin_savepoint(true, 2024040401, 'qtype', 'ordering'); + } + return true; } - - diff --git a/question/type/ordering/edit_ordering_form.php b/question/type/ordering/edit_ordering_form.php index 635e17b3c27..9e5b9b2407c 100644 --- a/question/type/ordering/edit_ordering_form.php +++ b/question/type/ordering/edit_ordering_form.php @@ -98,14 +98,14 @@ class qtype_ordering_edit_form extends question_edit_form { // Field for selectcount. $name = 'selectcount'; $label = get_string($name, $plugin); - $options = [0 => get_string('all')]; - for ($i = 3; $i <= 20; $i++) { - $options[$i] = $i; - } - $mform->addElement('select', $name, $label, $options); - $mform->disabledIf($name, 'selecttype', 'eq', 0); + + $mform->addElement('text', $name, $label, ['size' => 2]); + $mform->setDefault($name, qtype_ordering_question::MIN_SUBSET_ITEMS); + $mform->setType($name, PARAM_INT); + // Hide the field if 'Item selection type' is set to select all items. + $mform->hideIf($name, 'selecttype', 'eq', qtype_ordering_question::SELECT_ALL); $mform->addHelpButton($name, $name, $plugin); - $mform->setDefault($name, 6); + $mform->addRule($name, null, 'numeric', null, 'client'); // Field for gradingtype. $name = 'gradingtype'; @@ -133,13 +133,12 @@ class qtype_ordering_edit_form extends question_edit_form { $elements = []; $options = []; - $name = 'answerheader'; - $label = get_string($name, $plugin); - $elements[] = $mform->createElement('header', $name, $label); - $options[$name] = ['expanded' => true]; + $mform->addElement('header', 'answersheader', get_string('draggableitems', $plugin)); + $mform->setExpanded('answersheader', true); $name = 'answer'; - $elements[] = $mform->createElement('editor', $name, $label, $this->get_editor_attributes(), $this->get_editor_options()); + $elements[] = $mform->createElement('editor', $name, get_string('draggableitemno', $plugin), + $this->get_editor_attributes(), $this->get_editor_options()); $elements[] = $mform->createElement('submit', $name . 'removeeditor', get_string('removeeditor', $plugin), ['onclick' => 'skipClientValidation = true;']); $options[$name] = ['type' => PARAM_RAW]; @@ -372,7 +371,7 @@ class qtype_ordering_edit_form extends question_edit_form { $names = [ 'layouttype' => qtype_ordering_question::LAYOUT_VERTICAL, 'selecttype' => qtype_ordering_question::SELECT_ALL, - 'selectcount' => 0, // 0 means ALL. + 'selectcount' => qtype_ordering_question::MIN_SUBSET_ITEMS, 'gradingtype' => qtype_ordering_question::GRADING_ABSOLUTE_POSITION, 'showgrading' => 1, // 1 means SHOW. 'numberingstyle' => qtype_ordering_question::NUMBERING_STYLE_DEFAULT, @@ -418,6 +417,12 @@ class qtype_ordering_edit_form extends question_edit_form { $errors = []; $plugin = 'qtype_ordering'; + $minsubsetitems = qtype_ordering_question::MIN_SUBSET_ITEMS; + // Make sure the entered size of the subset is no less than the defined minimum. + if ($data['selecttype'] != qtype_ordering_question::SELECT_ALL && $data['selectcount'] < $minsubsetitems) { + $errors['selectcount'] = get_string('notenoughsubsetitems', $plugin, $minsubsetitems); + } + // Identify duplicates and report as an error. $answers = []; $answercount = 0; @@ -428,9 +433,9 @@ class qtype_ordering_edit_form extends question_edit_form { if ($answer = trim($answer)) { if (in_array($answer, $answers)) { $i = array_search($answer, $answers); - $item = get_string('answerheader', $plugin); + $item = get_string('draggableitemno', $plugin); $item = str_replace('{no}', $i + 1, $item); - $item = html_writer::link("#id_answerheader_$i", $item); + $item = html_writer::link("#id_answer_$i", $item); $a = (object) ['text' => $answer, 'item' => $item]; $errors["answer[$answercount]"] = get_string('duplicatesnotallowed', $plugin, $a); } else { diff --git a/question/type/ordering/lang/en/qtype_ordering.php b/question/type/ordering/lang/en/qtype_ordering.php index 5d8ca1923bf..b7ed6806547 100644 --- a/question/type/ordering/lang/en/qtype_ordering.php +++ b/question/type/ordering/lang/en/qtype_ordering.php @@ -27,7 +27,6 @@ $string['addmultipleanswers'] = 'Add {$a} more items'; $string['addsingleanswer'] = 'Add one more item'; $string['allornothing'] = 'All or nothing'; $string['answer'] = 'Item text'; -$string['answerheader'] = 'Draggable item {no}'; $string['correctorder'] = 'The correct order for these items is as follows:'; @@ -35,6 +34,8 @@ $string['defaultanswerformat'] = 'Default answer format'; $string['defaultquestionname'] = 'Drag the following items into the correct order.'; $string['duplicatesnotallowed'] = 'Duplication of draggable items is not allowed. The string "{$a->text}" is already used in {$a->item}.'; +$string['draggableitems'] = 'Draggable items'; +$string['draggableitemno'] = 'Draggable item {no}'; $string['editingordering'] = 'Editing ordering question'; $string['gradedetails'] = 'Grade details'; @@ -79,6 +80,7 @@ $string['longestorderedsubset'] = 'Longest ordered subset'; $string['noresponsedetails'] = 'Sorry, no details of the response to this question are available.'; $string['noscore'] = 'No score'; $string['notenoughanswers'] = 'Ordering questions must have more than {$a} answers.'; +$string['notenoughsubsetitems'] = 'A subset must have at least {$a} items.'; $string['numberingstyle'] = 'Number the choices?'; $string['numberingstylenone'] = 'No numbering'; diff --git a/question/type/ordering/question.php b/question/type/ordering/question.php index 6411cfb011d..1fff8da6e91 100644 --- a/question/type/ordering/question.php +++ b/question/type/ordering/question.php @@ -45,6 +45,9 @@ class qtype_ordering_question extends question_graded_automatically { /** Show answers in one horizontal line */ const LAYOUT_HORIZONTAL = 1; + /** The minimum number of items to create a subset */ + const MIN_SUBSET_ITEMS = 2; + /** Default value for numberingstyle */ const NUMBERING_STYLE_DEFAULT = 'none'; @@ -137,7 +140,7 @@ class qtype_ordering_question extends question_graded_automatically { // Sanitize "selectcount". $selectcount = $this->selectcount; - $selectcount = max(3, $selectcount); + $selectcount = max(self::MIN_SUBSET_ITEMS, $selectcount); $selectcount = min($countanswers, $selectcount); // Ensure consistency between "selecttype" and "selectcount". diff --git a/question/type/ordering/questiontype.php b/question/type/ordering/questiontype.php index 9ecef0dd321..1fbb04c5908 100644 --- a/question/type/ordering/questiontype.php +++ b/question/type/ordering/questiontype.php @@ -513,7 +513,8 @@ class qtype_ordering extends question_type { $question->qtype = 'ordering'; // Set "selectcount" field from $selectcount. - if (is_numeric($selectcount) && $selectcount > 2 && $selectcount <= count($answers)) { + if (is_numeric($selectcount) && $selectcount >= qtype_ordering_question::MIN_SUBSET_ITEMS && + $selectcount <= count($answers)) { $selectcount = intval($selectcount); } else { $selectcount = min(6, count($answers)); @@ -843,10 +844,10 @@ class qtype_ordering extends question_type { }; // Set "selectcount" option - this used to be ($count - 2). - if (is_numeric($selectcount)) { + if (is_numeric($selectcount) && $selectcount >= qtype_ordering_question::MIN_SUBSET_ITEMS) { $question->selectcount = intval($selectcount); } else { - $question->selectcount = 3; // Default! + $question->selectcount = qtype_ordering_question::MIN_SUBSET_ITEMS; // Default! } // Set "gradingtype" option. diff --git a/question/type/ordering/tests/fixtures/testoldquestion.moodle.xml b/question/type/ordering/tests/fixtures/testoldquestion.moodle.xml index 7938e751cb9..179f8bd8abd 100644 --- a/question/type/ordering/tests/fixtures/testoldquestion.moodle.xml +++ b/question/type/ordering/tests/fixtures/testoldquestion.moodle.xml @@ -25,7 +25,7 @@ dd1 HORIZONTAL ALL - 0 + 2 ABSOLUTE_POSITION SHOW diff --git a/question/type/ordering/tests/fixtures/testquestion.moodle.xml b/question/type/ordering/tests/fixtures/testquestion.moodle.xml index 7c214923fb8..ae73f848f23 100644 --- a/question/type/ordering/tests/fixtures/testquestion.moodle.xml +++ b/question/type/ordering/tests/fixtures/testquestion.moodle.xml @@ -25,7 +25,7 @@ dd1 HORIZONTAL ALL - 0 + 2 ABSOLUTE_POSITION SHOW diff --git a/question/type/ordering/tests/helper.php b/question/type/ordering/tests/helper.php index cdf71656dc3..a942075d90a 100644 --- a/question/type/ordering/tests/helper.php +++ b/question/type/ordering/tests/helper.php @@ -64,7 +64,7 @@ class qtype_ordering_test_helper extends question_test_helper { ]; $q->layouttype = qtype_ordering_question::LAYOUT_HORIZONTAL; $q->selecttype = qtype_ordering_question::SELECT_ALL; - $q->selectcount = 0; + $q->selectcount = qtype_ordering_question::MIN_SUBSET_ITEMS; $q->gradingtype = qtype_ordering_question::GRADING_RELATIVE_ALL_PREVIOUS_AND_NEXT; $q->showgrading = true; $q->numberingstyle = qtype_ordering_question::NUMBERING_STYLE_DEFAULT; @@ -126,7 +126,7 @@ class qtype_ordering_test_helper extends question_test_helper { $form->layouttype = qtype_ordering_question::LAYOUT_HORIZONTAL; $form->selecttype = qtype_ordering_question::SELECT_ALL; - $form->selectcount = 0; + $form->selectcount = qtype_ordering_question::MIN_SUBSET_ITEMS; $form->gradingtype = qtype_ordering_question::GRADING_RELATIVE_ALL_PREVIOUS_AND_NEXT; $form->showgrading = true; $form->numberingstyle = qtype_ordering_question::NUMBERING_STYLE_DEFAULT; @@ -170,7 +170,7 @@ class qtype_ordering_test_helper extends question_test_helper { test_question_maker::set_standard_combined_feedback_fields($questiondata->options); $questiondata->options->layouttype = qtype_ordering_question::LAYOUT_HORIZONTAL; $questiondata->options->selecttype = qtype_ordering_question::SELECT_ALL; - $questiondata->options->selectcount = 0; + $questiondata->options->selectcount = qtype_ordering_question::MIN_SUBSET_ITEMS; $questiondata->options->gradingtype = qtype_ordering_question::GRADING_RELATIVE_ALL_PREVIOUS_AND_NEXT; $questiondata->options->showgrading = true; $questiondata->options->numberingstyle = qtype_ordering_question::NUMBERING_STYLE_DEFAULT; diff --git a/question/type/ordering/tests/output/specific_grade_detail_feedback_test.php b/question/type/ordering/tests/output/specific_grade_detail_feedback_test.php index c56dc450656..49c043f2f80 100644 --- a/question/type/ordering/tests/output/specific_grade_detail_feedback_test.php +++ b/question/type/ordering/tests/output/specific_grade_detail_feedback_test.php @@ -203,7 +203,7 @@ class specific_grade_detail_feedback_test extends advanced_testcase { 'orderinglayoutclass' => 'vertical', 'gradedetails' => 0, 'totalscore' => 0, - 'totalmaxscore' => 3, + 'totalmaxscore' => 2, 'scoredetails' => [ ['score' => 0, 'maxscore' => 1, 'percent' => 0.0], ['score' => 0, 'maxscore' => 1, 'percent' => 0.0], @@ -225,7 +225,7 @@ class specific_grade_detail_feedback_test extends advanced_testcase { 'orderinglayoutclass' => 'vertical', 'gradedetails' => 0, 'totalscore' => 0, - 'totalmaxscore' => 3, + 'totalmaxscore' => 2, 'scoredetails' => [ ['score' => 0, 'maxscore' => 1, 'percent' => 0], ['score' => 1, 'maxscore' => 1, 'percent' => 100.0], diff --git a/question/type/ordering/version.php b/question/type/ordering/version.php index 1f83cadedb5..52320bc02ae 100644 --- a/question/type/ordering/version.php +++ b/question/type/ordering/version.php @@ -29,4 +29,4 @@ $plugin->cron = 0; $plugin->component = 'qtype_ordering'; $plugin->maturity = MATURITY_STABLE; $plugin->requires = 2021051700; // Moodle 3.11. -$plugin->version = 2024040400; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2024040401; // The current plugin version (Date: YYYYMMDDXX).