From cf62f5cc76c727d9779abd58cd37d8067d152dc5 Mon Sep 17 00:00:00 2001 From: Philipp Imhof <52650214+PhilippImhof@users.noreply.github.com> Date: Sat, 16 Sep 2023 16:57:43 +0200 Subject: [PATCH] MDL-67335 qtype_calculatedmulti: allow HTML in answer options --- question/format/xml/format.php | 36 ++++++-- question/type/calculated/styles.css | 6 +- question/type/calculatedmulti/db/upgrade.php | 51 ++++++++++ .../edit_calculatedmulti_form.php | 44 ++++----- .../type/calculatedmulti/questiontype.php | 48 ++++++---- .../calculatedmulti/tests/behat/edit.feature | 92 +++++++++++++++++++ question/type/calculatedmulti/upgrade.txt | 5 + question/type/calculatedmulti/version.php | 2 +- 8 files changed, 227 insertions(+), 57 deletions(-) create mode 100644 question/type/calculatedmulti/db/upgrade.php create mode 100644 question/type/calculatedmulti/tests/behat/edit.feature create mode 100644 question/type/calculatedmulti/upgrade.txt diff --git a/question/format/xml/format.php b/question/format/xml/format.php index 4ba97f2836f..90d2f1ffc14 100644 --- a/question/format/xml/format.php +++ b/question/format/xml/format.php @@ -160,6 +160,7 @@ class qformat_xml extends qformat_default { if ($istext) { if (!is_string($xml)) { $this->error(get_string('invalidxml', 'qformat_xml')); + return false; } $xml = trim($xml); } @@ -863,7 +864,21 @@ class qformat_xml extends qformat_default { if (empty($ans->answer['text'])) { $ans->answer['text'] = '*'; } - $qo->answer[] = $ans->answer['text']; + // The qtype_calculatedmulti allows HTML in answer options. + if ($question['@']['type'] == 'calculatedmulti') { + // If the import file contains a "format" attribute for the answer text, + // then use it. Otherwise, we must set the answerformat to FORMAT_PLAIN, + // because the question has been exported from a Moodle version that + // did not yet allow HTML answer options. + if (array_key_exists('format', $answer['@'])) { + $ans->answer['format'] = $this->trans_format($answer['@']['format']); + } else { + $ans->answer['format'] = FORMAT_PLAIN; + } + $qo->answer[] = $ans->answer; + } else { + $qo->answer[] = $ans->answer['text']; + } $qo->feedback[] = $ans->feedback; $qo->tolerance[] = $answer['#']['tolerance'][0]['#']; // Fraction as a tag is deprecated. @@ -901,7 +916,7 @@ class qformat_xml extends qformat_default { } } - $datasets = $question['#']['dataset_definitions'][0]['#']['dataset_definition']; + $datasets = $question['#']['dataset_definitions'][0]['#']['dataset_definition'] ?? []; $qo->dataset = array(); $qo->datasetindex= 0; foreach ($datasets as $dataset) { @@ -1439,12 +1454,19 @@ class qformat_xml extends qformat_default { foreach ($question->options->answers as $answer) { $percent = 100 * $answer->fraction; - $expout .= " \n"; + // For qtype_calculatedmulti, answer options (choices) can be in plain text or in HTML + // format, so we need to specify when exporting a question. + if ($component == 'qtype_calculatedmulti') { + $expout .= "format($answer->answerformat)}>\n"; + } else { + $expout .= "\n"; + } // The "" tags are an added feature, old files won't have them. - $expout .= $this->writetext($answer->answer, 3); - $expout .= " {$answer->tolerance}\n"; - $expout .= " {$answer->tolerancetype}\n"; - $expout .= " " . + $expout .= $this->writetext($answer->answer); + $expout .= $this->write_files($this->answerfiles[$answer->id]); + $expout .= " {$answer->tolerance}\n"; + $expout .= " {$answer->tolerancetype}\n"; + $expout .= " " . $answer->correctanswerformat . "\n"; $expout .= " " . $answer->correctanswerlength . "\n"; diff --git a/question/type/calculated/styles.css b/question/type/calculated/styles.css index 4fb5884f0e7..c5d8d743f41 100644 --- a/question/type/calculated/styles.css +++ b/question/type/calculated/styles.css @@ -21,7 +21,7 @@ /* Editing form. */ body#page-question-type-calculated div[id^=fgroup_id_][id*=answeroptions_], -body#page-question-type-calculatedmulti div[id^=fgroup_id_][id*=answeroptions_] { +body#page-question-type-calculatedmulti div[id^=fitem_id_][id*=answer_] { background: #eee; margin-top: 0; margin-bottom: 0; @@ -32,14 +32,13 @@ body#page-question-type-calculatedmulti div[id^=fgroup_id_][id*=answeroptions_] } body#page-question-type-calculated div[id^=fgroup_id_][id*=answeroptions_] .fgrouplabel label, -body#page-question-type-calculatedmulti div[id^=fgroup_id_][id*=answeroptions_] .fgrouplabel label { +body#page-question-type-calculatedmulti div[id^=fitem_id_][id*=answer_] .fgrouplabel label { font-weight: bold; } body#page-question-type-calculated div[id^=fgroup_id_][id*=answeroptions_] label[for^='id_answer_'], body#page-question-type-calculated div[id^=fgroup_id_][id*=answertolerance_] label[for^='id_tolerance_'], body#page-question-type-calculated div[id^=fgroup_id_][id*=answerdisplay_] label[for^='id_correctanswerlength_'], -body#page-question-type-calculatedmulti div[id^=fgroup_id_][id*=answeroptions_] label[for^='id_answer_'], body#page-question-type-calculatedmulti div[id^=fgroup_id_][id*=answerdisplay_] label[for^='id_correctanswerlength_'] { position: absolute; left: -10000px; @@ -49,6 +48,7 @@ body#page-question-type-calculatedmulti div[id^=fgroup_id_][id*=answerdisplay_] body#page-question-type-calculated div[id^=fgroup_id_][id*=answertolerance_], body#page-question-type-calculated div[id^=fgroup_id_][id*=answerdisplay_], +body#page-question-type-calculatedmulti div[id^=fitem_id_][id*=fraction_], body#page-question-type-calculatedmulti div[id^=fgroup_id_][id*=answertolerance_], body#page-question-type-calculatedmulti div[id^=fgroup_id_][id*=answerdisplay_] { background: #eee; diff --git a/question/type/calculatedmulti/db/upgrade.php b/question/type/calculatedmulti/db/upgrade.php new file mode 100644 index 00000000000..111dc1238e4 --- /dev/null +++ b/question/type/calculatedmulti/db/upgrade.php @@ -0,0 +1,51 @@ +. + +/** + * Calculated multiple-choice question type upgrade code. + * + * @package qtype_calculatedmulti + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +/** + * Upgrade code for the calculatedmulti question type. + * @param int $oldversion the version we are upgrading from. + * @return bool + */ +function xmldb_qtype_calculatedmulti_upgrade($oldversion) { + global $DB; + + if ($oldversion < 2024011700) { + // In earlier versions, the answer options (choices) for a calculated multiple choice + // question could not use HTML and all text was rendered verbatim. However, the texts + // were stored in the DB with answerformat == FORMAT_HTML. This value was then overridden + // during initialisation of the question. + // From this version on, answer options may use HTML, so the answerformat does now have a + // meaning. For backwards compatibility, all existing answer options for this question + // type must have their answerformat set to FORMAT_PLAIN. + $DB->execute("UPDATE {question_answers} AS answers, {question} AS questions + SET answerformat = '" . FORMAT_PLAIN . "' + WHERE answers.question = questions.id + AND questions.qtype = 'calculatedmulti'"); + + // Calculatedmulti savepoint reached. + upgrade_plugin_savepoint(true, 2024011700, 'qtype', 'calculatedmulti'); + } + + return true; +} diff --git a/question/type/calculatedmulti/edit_calculatedmulti_form.php b/question/type/calculatedmulti/edit_calculatedmulti_form.php index 010729a14e2..c9fa3dea057 100644 --- a/question/type/calculatedmulti/edit_calculatedmulti_form.php +++ b/question/type/calculatedmulti/edit_calculatedmulti_form.php @@ -68,21 +68,13 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { public function get_per_answer_fields($mform, $label, $gradeoptions, &$repeatedoptions, &$answersoption) { $repeated = array(); - $answeroptions = array(); - $answeroptions[] = $mform->createElement('text', 'answer', - $label, array('size' => 50)); - $answeroptions[] = $mform->createElement('select', 'fraction', - get_string('gradenoun'), $gradeoptions); - $repeated[] = $mform->createElement('group', 'answeroptions', - $label, $answeroptions, null, false); - - // Added answeroptions help button in definition_inner() after called to add_per_answer_fields. + // Help button will be added in definition_inner() after called to add_per_answer_fields. + $repeated[] = $mform->createElement('editor', 'answer', $label, null, $this->editoroptions); $repeatedoptions['answer']['type'] = PARAM_RAW; - $repeatedoptions['fraction']['default'] = 0; - $answersoption = 'answers'; - $mform->setType('answer', PARAM_NOTAGS); + $repeated[] = $mform->createElement('select', 'fraction', get_string('gradenoun'), $gradeoptions); + $repeatedoptions['fraction']['default'] = 0; $repeated[] = $mform->createElement('hidden', 'tolerance'); $repeated[] = $mform->createElement('hidden', 'tolerancetype', 1); @@ -92,7 +84,7 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { // Create display group. $answerdisplay = array(); - $answerdisplay[] = $mform->createElement('select', 'correctanswerlength', + $answerdisplay[] = $mform->createElement('select', 'correctanswerlength', get_string('answerdisplay', 'qtype_calculated'), range(0, 9)); $repeatedoptions['correctanswerlength']['default'] = 2; @@ -109,6 +101,7 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { $repeated[] = $mform->createElement('editor', 'feedback', get_string('feedback', 'question'), null, $this->editoroptions); + $answersoption = 'answers'; return $repeated; } @@ -156,9 +149,8 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { $this->add_per_answer_fields($mform, get_string('choiceno', 'qtype_multichoice', '{no}'), question_bank::fraction_options_full(), max(5, QUESTION_NUMANS_START)); - $mform->addHelpButton('answeroptions[0]', 'answeroptions', 'qtype_calculatedmulti'); + $mform->addHelpButton('answer[0]', 'answeroptions', 'qtype_calculatedmulti'); - $repeated = array(); $nounits = optional_param('nounits', 1, PARAM_INT); $mform->addElement('hidden', 'nounits', $nounits); $mform->setType('nounits', PARAM_INT); @@ -191,7 +183,7 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { public function data_preprocessing($question) { $question = parent::data_preprocessing($question); - $question = $this->data_preprocessing_answers($question, false); + $question = $this->data_preprocessing_answers($question, true); $question = $this->data_preprocessing_combined_feedback($question, true); $question = $this->data_preprocessing_hints($question, true, true); @@ -260,11 +252,11 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { $possibledatasets = $this->qtypeobj->find_dataset_names($data['questiontext']['text']); $mandatorydatasets = array(); foreach ($answers as $key => $answer) { - $mandatorydatasets += $this->qtypeobj->find_dataset_names($answer); + $mandatorydatasets += $this->qtypeobj->find_dataset_names($answer['text']); } if (count($mandatorydatasets) == 0) { foreach ($answers as $key => $answer) { - $errors['answeroptions['.$key.']'] = + $errors['answer['.$key.']'] = get_string('atleastonewildcard', 'qtype_calculated'); } } @@ -272,17 +264,17 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { $totalfraction = 0; $maxfraction = -1; foreach ($answers as $key => $answer) { - $trimmedanswer = trim($answer); + $trimmedanswer = trim($answer['text']); $fraction = (float) $data['fraction'][$key]; if (empty($trimmedanswer) && $trimmedanswer != '0' && empty($fraction)) { continue; } if (empty($trimmedanswer)) { - $errors['answeroptions['.$key.']'] = get_string('errgradesetanswerblank', 'qtype_multichoice'); + $errors['answer['.$key.']'] = get_string('errgradesetanswerblank', 'qtype_multichoice'); } if ($trimmedanswer != '' || $answercount == 0) { // Verifying for errors in {=...} in answer text. - $errors = $this->validate_text($errors, 'answeroptions[' . $key . ']', $answer); + $errors = $this->validate_text($errors, 'answer[' . $key . ']', $answer['text']); $errors = $this->validate_text($errors, 'feedback[' . $key . ']', $data['feedback'][$key]['text']); } @@ -309,23 +301,23 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { } } if ($answercount == 0) { - $errors['answeroptions[0]'] = get_string('notenoughanswers', 'qtype_multichoice', 2); - $errors['answeroptions[1]'] = get_string('notenoughanswers', 'qtype_multichoice', 2); + $errors['answer[0]'] = get_string('notenoughanswers', 'qtype_multichoice', 2); + $errors['answer[1]'] = get_string('notenoughanswers', 'qtype_multichoice', 2); } else if ($answercount == 1) { - $errors['answeroptions[1]'] = get_string('notenoughanswers', 'qtype_multichoice', 2); + $errors['answer[1]'] = get_string('notenoughanswers', 'qtype_multichoice', 2); } // Perform sanity checks on fractional grades. if ($data['single']== 1 ) { if ($maxfraction != 1) { - $errors['answeroptions[0]'] = get_string('errfractionsnomax', 'qtype_multichoice', + $errors['answer[0]'] = get_string('errfractionsnomax', 'qtype_multichoice', $maxfraction * 100); } } else { $totalfraction = round($totalfraction, 2); if ($totalfraction != 1) { $totalfraction = $totalfraction * 100; - $errors['answeroptions[0]'] = + $errors['answer[0]'] = get_string('errfractionsaddwrong', 'qtype_multichoice', $totalfraction); } } diff --git a/question/type/calculatedmulti/questiontype.php b/question/type/calculatedmulti/questiontype.php index f831204c5ef..2bef4fb7544 100644 --- a/question/type/calculatedmulti/questiontype.php +++ b/question/type/calculatedmulti/questiontype.php @@ -40,14 +40,28 @@ require_once($CFG->dirroot . '/question/type/calculated/questiontype.php'); class qtype_calculatedmulti extends qtype_calculated { public function save_question_options($question) { - global $CFG, $DB; + global $DB; $context = $question->context; + // During validation, we will call the parent's validation method. It will check + // the answer options (choices) for formula errors. This method is shared with + // calculated and calculatedsimple types for which the text is just a formula and + // thus plain text from a regular text field. Therefore, we must transform the answers + // before sending them upstream. We save the original data for later. + $question->originalanswer = []; + foreach ($question->answer as $key => $answerdata) { + $question->originalanswer[$key] = $answerdata; + if (is_array($answerdata)) { + $question->answer[$key] = $answerdata['text']; + } else { + $question->answer[$key] = $answerdata; + } + } + // Make it impossible to save bad formulas anywhere. $this->validate_question_data($question); // Calculated options. - $update = true; $options = $DB->get_record('question_calculated_options', array('question' => $question->id)); if (!$options) { @@ -76,11 +90,16 @@ class qtype_calculatedmulti extends qtype_calculated { } // Insert all the new answers. - foreach ($question->answer as $key => $answerdata) { + foreach ($question->originalanswer as $key => $answerdata) { if (is_array($answerdata)) { - $answerdata = $answerdata['text']; + $answertext = $answerdata['text']; + $answerformat = $answerdata['format']; + } else { + $answertext = $answerdata; + // If no format is set, assume it is a legacy question and use FORMAT_PLAIN. + $answerformat = FORMAT_PLAIN; } - if (trim($answerdata) == '') { + if (trim($answertext) == '') { continue; } @@ -94,16 +113,9 @@ class qtype_calculatedmulti extends qtype_calculated { $answer->id = $DB->insert_record('question_answers', $answer); } - if (is_array($answerdata)) { - // Doing an import. - $answer->answer = $this->import_or_save_files($answerdata, - $context, 'question', 'answer', $answer->id); - $answer->answerformat = $answerdata['format']; - } else { - // Saving the form. - $answer->answer = $answerdata; - $answer->answerformat = FORMAT_HTML; - } + $answer->answer = $this->import_or_save_files($question->originalanswer[$key], $context, + 'question', 'answer', $answer->id); + $answer->answerformat = $answerformat; $answer->fraction = $question->fraction[$key]; $answer->feedback = $this->import_or_save_files($question->feedback[$key], $context, 'question', 'answerfeedback', $answer->id); @@ -149,10 +161,6 @@ class qtype_calculatedmulti extends qtype_calculated { if (isset($question->import_process) && $question->import_process) { $this->import_datasets($question); } - // Report any problems. - if (!empty($result->notice)) { - return $result; - } return true; } @@ -195,7 +203,7 @@ class qtype_calculatedmulti extends qtype_calculated { $question->synchronised = $questiondata->options->synchronize; $this->initialise_combined_feedback($question, $questiondata, true); - $this->initialise_question_answers($question, $questiondata); + $this->initialise_question_answers($question, $questiondata, false); foreach ($questiondata->options->answers as $a) { $question->answers[$a->id]->correctanswerlength = $a->correctanswerlength; diff --git a/question/type/calculatedmulti/tests/behat/edit.feature b/question/type/calculatedmulti/tests/behat/edit.feature new file mode 100644 index 00000000000..01067c9320f --- /dev/null +++ b/question/type/calculatedmulti/tests/behat/edit.feature @@ -0,0 +1,92 @@ +@qtype @qtype_calculatedmulti +Feature: Test editing a Calculated multichoice question + As a teacher + In order to be able to update my Calculated multichoice questions + I need to edit them + + Background: + Given the following "users" exist: + | username | + | teacher | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher | C1 | editingteacher | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "user preferences" exist: + | user | preference | value | + | teacher | htmleditor | textarea | + + Scenario: Add, edit and preview a Calculated multichoice question with HTML in answers + When I am on the "Course 1" "core_question > course question bank" page logged in as teacher + And I press "Create a new question ..." + And I set the field "Calculated multichoice" to "1" + And I click on "Add" "button" + And I set the following fields to these values: + | Question name | calculatedmulti-001 | + | Question text | Multiply those two: s^{A} and s{B} | + | Choice 1 | s{={A}*{B}} | + | Choice 1 format | 1 | + | Grade | 100% | + | Choice 2 | s{={A}+{B}} | + | Choice 2 format | 1 | + And I press "id_submitbutton" + And I should see "Choose wildcards dataset properties" + And I press "id_submitbutton" + And I should see "Edit the wildcards datasets" + And I press "id_addbutton" + And I set the following fields to these values: + | id_number_2 | 6 | + | id_number_1 | 4 | + And I press "id_savechanges" + # Checking that the wildcard values are there + And I am on the "calculatedmulti-001" "core_question > edit" page logged in as teacher + And I set the following fields to these values: + | Question name | Edited question name | + And I press "id_submitbutton" + And I should see "Choose wildcards dataset properties" + And I press "id_submitbutton" + And I press "id_savechanges" + And I should see "Edited question name" + # Preview it. + And I choose "Preview" action for "Edited question name" in the question bank + Then I should not see "" + + Scenario: Add, edit and preview a Calculated multichoice question with plain-text answers + When I am on the "Course 1" "core_question > course question bank" page logged in as teacher + And I press "Create a new question ..." + And I set the field "Calculated multichoice" to "1" + And I click on "Add" "button" + And I set the following fields to these values: + | Question name | calculatedmulti-001 | + | Question text | Multiply those two: s^{A} and s{B} | + | Choice 1 | s{={A}*{B}} | + | Choice 1 format | 2 | + | Grade | 100% | + | Choice 2 | s{={A}+{B}} | + | Choice 2 format | 2 | + And I press "id_submitbutton" + And I should see "Choose wildcards dataset properties" + And I press "id_submitbutton" + And I should see "Edit the wildcards datasets" + And I press "id_addbutton" + And I set the following fields to these values: + | id_number_2 | 6 | + | id_number_1 | 4 | + And I press "id_savechanges" + # Checking that the wildcard values are there + And I am on the "calculatedmulti-001" "core_question > edit" page logged in as teacher + And I set the following fields to these values: + | Question name | Edited question name | + And I press "id_submitbutton" + And I should see "Choose wildcards dataset properties" + And I press "id_submitbutton" + And I press "id_savechanges" + And I should see "Edited question name" + # Preview it. + And I choose "Preview" action for "Edited question name" in the question bank + Then I should see "" diff --git a/question/type/calculatedmulti/upgrade.txt b/question/type/calculatedmulti/upgrade.txt new file mode 100644 index 00000000000..dadfd9b21b8 --- /dev/null +++ b/question/type/calculatedmulti/upgrade.txt @@ -0,0 +1,5 @@ +This file describes API changes in /question/type/calculatedmulti/*. + +=== 4.4 === +* Calculated multichoice question options now have the possibility to have full HTML (and MathJax) support in choices the same + as normal multichoice question options. \ No newline at end of file diff --git a/question/type/calculatedmulti/version.php b/question/type/calculatedmulti/version.php index aa934afba92..9285b507f93 100644 --- a/question/type/calculatedmulti/version.php +++ b/question/type/calculatedmulti/version.php @@ -26,7 +26,7 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'qtype_calculatedmulti'; -$plugin->version = 2023100900; +$plugin->version = 2024011700; $plugin->requires = 2023100400; $plugin->dependencies = [