mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-17 06:25:33 +02:00
Code Issues Projects Releases Wiki Activity

Merge branch 'MDL-63905' of git://github.com/jmvedrine/moodle

Browse Source
This commit is contained in:
Adrian Greeve 2019-03-07 09:09:23 +08:00
commit ebdd345054
18 changed files with 305 additions and 169 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
question/format/multianswer/format.php
View File

@ -51,7 +51,14 @@ class qformat_multianswer extends qformat_default {
$questiontext['text'] = implode('', $lines);
$questiontext['format'] = FORMAT_MOODLE;
$questiontext['itemid'] = '';
$question = qtype_multianswer_extract_question($questiontext);
$errors = qtype_multianswer_validate_question($question);
if ($errors) {
$this->error(get_string('invalidmultianswerquestion', 'qtype_multianswer', implode(' ', $errors)));
return array();
}
$question->questiontext = $question->questiontext['text'];
$question->questiontextformat = 0;

1
question/format/multianswer/tests/fixtures/broken_multianswer_1.txt vendored Normal file
View File

@ -0,0 +1 @@
Please select the fruits {1:MULTICHOICE:=Apple#Correct}

1
question/format/multianswer/tests/fixtures/broken_multianswer_2.txt vendored Normal file
View File

@ -0,0 +1 @@
Please select the fruits {1:MULTICHOICE:Pear#Incorrect~%50%Apple#Correct}

1
question/format/multianswer/tests/fixtures/broken_multianswer_3.txt vendored Normal file
View File

@ -0,0 +1 @@
What grade would you give it? {3:NUMERICAL:=zero}

1
question/format/multianswer/tests/fixtures/broken_multianswer_4.txt vendored Normal file
View File

@ -0,0 +1 @@
Please select the fruits.

77
question/format/multianswer/tests/multianswerformat_test.php
View File

@ -75,4 +75,81 @@ The capital of France is {#5}.
$this->assertEquals('multichoice', $qs[0]->options->questions[4]->qtype);
$this->assertEquals('shortanswer', $qs[0]->options->questions[5]->qtype);
}
public function test_read_brokencloze_1() {
$lines = file(__DIR__ . '/fixtures/broken_multianswer_1.txt');
$importer = new qformat_multianswer();
// The importer echoes some errors, so we need to capture and check that.
ob_start();
$questions = $importer->readquestions($lines);
$output = ob_get_contents();
ob_end_clean();
// Check that there were some expected errors.
$this->assertContains('Error importing question', $output);
$this->assertContains('Invalid embedded answers (Cloze) question', $output);
$this->assertContains('This type of question requires at least 2 choices', $output);
// No question have been imported.
$this->assertCount(0, $questions);
}
public function test_read_brokencloze_2() {
$lines = file(__DIR__ . '/fixtures/broken_multianswer_2.txt');
$importer = new qformat_multianswer();
// The importer echoes some errors, so we need to capture and check that.
ob_start();
$questions = $importer->readquestions($lines);
$output = ob_get_contents();
ob_end_clean();
// Check that there were some expected errors.
$this->assertContains('Error importing question', $output);
$this->assertContains('Invalid embedded answers (Cloze) question', $output);
$this->assertContains('One of the answers should have a score of 100% so it is possible to get full marks for this question.',
$output);
// No question have been imported.
$this->assertCount(0, $questions);
}
public function test_read_brokencloze_3() {
$lines = file(__DIR__ . '/fixtures/broken_multianswer_3.txt');
$importer = new qformat_multianswer();
// The importer echoes some errors, so we need to capture and check that.
ob_start();
$questions = $importer->readquestions($lines);
$output = ob_get_contents();
ob_end_clean();
// Check that there were some expected errors.
$this->assertContains('Error importing question', $output);
$this->assertContains('Invalid embedded answers (Cloze) question', $output);
$this->assertContains('The answer must be a number, for example -1.234 or 3e8, or \'*\'.', $output);
// No question have been imported.
$this->assertCount(0, $questions);
}
public function test_read_brokencloze_4() {
$lines = file(__DIR__ . '/fixtures/broken_multianswer_4.txt');
$importer = new qformat_multianswer();
// The importer echoes some errors, so we need to capture and check that.
ob_start();
$questions = $importer->readquestions($lines);
$output = ob_get_contents();
ob_end_clean();
// Check that there were some expected errors.
$this->assertContains('Error importing question', $output);
$this->assertContains('Invalid embedded answers (Cloze) question', $output);
$this->assertContains('The question text must include at least one embedded answer.', $output);
// No question have been imported.
$this->assertCount(0, $questions);
}
}

19
question/format/xml/format.php
View File

@ -240,7 +240,7 @@ class qformat_xml extends qformat_default {
// Restore files in generalfeedback.
$generalfeedback = $this->import_text_with_files($question,
array('#', 'generalfeedback', 0), $qo->generalfeedback, $this->get_format($qo->questiontextformat));
array('#', 'generalfeedback', 0), '', $this->get_format($qo->questiontextformat));
$qo->generalfeedback = $generalfeedback['text'];
$qo->generalfeedbackformat = $generalfeedback['format'];
if (!empty($generalfeedback['itemid'])) {
@ -475,6 +475,11 @@ class qformat_xml extends qformat_default {
$questiontext = $this->import_text_with_files($question,
array('#', 'questiontext', 0));
$qo = qtype_multianswer_extract_question($questiontext);
$errors = qtype_multianswer_validate_question($qo);
if ($errors) {
$this->error(get_string('invalidmultianswerquestion', 'qtype_multianswer', implode(' ', $errors)));
return null;
}
// Header parts particular to multianswer.
$qo->qtype = 'multianswer';
@ -483,8 +488,12 @@ class qformat_xml extends qformat_default {
if (isset($this->course)) {
$qo->course = $this->course;
}
$qo->name = $this->clean_question_name($this->import_text($question['#']['name'][0]['#']['text']));
if (isset($question['#']['name'])) {
$qo->name = $this->clean_question_name($this->import_text($question['#']['name'][0]['#']['text']));
} else {
$qo->name = $this->create_default_question_name($qo->questiontext['text'],
get_string('questionname', 'question'));
}
$qo->questiontextformat = $questiontext['format'];
$qo->questiontext = $qo->questiontext['text'];
if (!empty($questiontext['itemid'])) {
@ -514,7 +523,7 @@ class qformat_xml extends qformat_default {
// Restore files in generalfeedback.
$generalfeedback = $this->import_text_with_files($question,
array('#', 'generalfeedback', 0), $qo->generalfeedback, $this->get_format($qo->questiontextformat));
array('#', 'generalfeedback', 0), '', $this->get_format($qo->questiontextformat));
$qo->generalfeedback = $generalfeedback['text'];
$qo->generalfeedbackformat = $generalfeedback['format'];
if (!empty($generalfeedback['itemid'])) {
@ -937,7 +946,7 @@ class qformat_xml extends qformat_default {
* @param stdClass $context
* @return array (of objects) question objects.
*/
protected function readquestions($lines) {
public function readquestions($lines) {
// We just need it as one big string.
$lines = implode('', $lines);

82
question/format/xml/tests/fixtures/broken_cloze_questions.xml vendored Normal file
View File

@ -0,0 +1,82 @@
<?xml version="1.0" encoding="UTF-8"?>
<quiz>
<!-- question: 1 -->
<question type="cloze">
<name>
<text>Multianswer question without subquestions</text>
</name>
<questiontext format="html">
<text>Please select the fruits.</text>
</questiontext>
<generalfeedback format="html">
<text></text>
</generalfeedback>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
</question>
<!-- question: 2 -->
<question type="cloze">
<name>
<text>Multichoice subquestion without choices</text>
</name>
<questiontext format="html">
<text>Please select the fruits {1:MULTICHOICE}</text>
</questiontext>
<generalfeedback format="html">
<text></text>
</generalfeedback>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
</question>
<!-- question: 3 -->
<question type="cloze">
<name>
<text>Multichoice subquestion with only one choice</text>
</name>
<questiontext format="html">
<text>Please select the fruits {1:MULTICHOICE:=Apple#Correct}</text>
</questiontext>
<generalfeedback format="html">
<text></text>
</generalfeedback>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
</question>
<!-- question: 4 -->
<question type="cloze">
<name>
<text>Multichoice subquestion with no completely correct answer</text>
</name>
<questiontext format="html">
<text>Please select the fruits {1:MULTICHOICE:Pear#Incorrect~%50%Apple#Correct}</text>
</questiontext>
<generalfeedback format="html">
<text></text>
</generalfeedback>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
</question>
<!-- question: 5 -->
<question type="cloze">
<name>
<text>Numerical subquestion with buggy answer</text>
</name>
<questiontext format="html">
<text>What grade would you give it? {3:NUMERICAL:=zero}</text>
</questiontext>
<generalfeedback format="html">
<text></text>
</generalfeedback>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
</question>
</quiz>

26
question/format/xml/tests/qformat_xml_import_export_test.php
View File

@ -422,4 +422,30 @@ class qformat_xml_import_export_test extends advanced_testcase {
$expectedxml = file_get_contents(__DIR__ . '/fixtures/nested_categories_with_questions.xml');
$this->assert_same_xml($expectedxml, $qformat->exportprocess());
}
/**
* Test that bad multianswer questions are not imported.
*/
public function test_import_broken_multianswer_questions() {
$lines = file(__DIR__ . '/fixtures/broken_cloze_questions.xml');
$importer = $qformat = new qformat_xml();
// The importer echoes some errors, so we need to capture and check that.
ob_start();
$questions = $importer->readquestions($lines);
$output = ob_get_contents();
ob_end_clean();
// Check that there were some expected errors.
$this->assertContains('Error importing question', $output);
$this->assertContains('Invalid embedded answers (Cloze) question', $output);
$this->assertContains('This type of question requires at least 2 choices', $output);
$this->assertContains('The answer must be a number, for example -1.234 or 3e8, or \'*\'.', $output);
$this->assertContains('One of the answers should have a score of 100% so it is possible to get full marks for this question.',
$output);
$this->assertContains('The question text must include at least one embedded answer.', $output);
// No question have been imported.
$this->assertCount(0, $questions);
}
}

68
question/type/multianswer/edit_multianswer_form.php
View File

@ -398,7 +398,7 @@ class qtype_multianswer_edit_form extends question_edit_form {
if ($trimmedanswer !== '') {
$answercount++;
if ($subquestion->qtype == 'numerical' &&
!($this->is_valid_number($trimmedanswer) || $trimmedanswer == '*')) {
!(qtype_numerical::is_valid_number($trimmedanswer) || $trimmedanswer == '*')) {
$this->_form->setElementError($prefix.'answer['.$key.']',
get_string('answermustbenumberorstar',
'qtype_numerical'));
@ -456,76 +456,12 @@ class qtype_multianswer_edit_form extends question_edit_form {
parent::set_data($question);
}
/**
* Validate that a string is a nubmer formatted correctly for the current locale.
* @param string $x a string
* @return bool whether $x is a number that the numerical question type can interpret.
*/
protected function is_valid_number($x) {
if (is_null($this->ap)) {
$this->ap = new qtype_numerical_answer_processor(array());
}
list($value, $unit) = $this->ap->apply_units($x);
return !is_null($value) && !$unit;
}
public function validation($data, $files) {
$errors = parent::validation($data, $files);
$questiondisplay = qtype_multianswer_extract_question($data['questiontext']);
if (isset($questiondisplay->options->questions)) {
$subquestions = fullclone($questiondisplay->options->questions);
if (count($subquestions)) {
$sub = 1;
foreach ($subquestions as $subquestion) {
$prefix = 'sub_'.$sub.'_';
$answercount = 0;
$maxgrade = false;
$maxfraction = -1;
foreach ($subquestion->answer as $key => $answer) {
if (is_array($answer)) {
$answer = $answer['text'];
}
$trimmedanswer = trim($answer);
if ($trimmedanswer !== '') {
$answercount++;
if ($subquestion->qtype == 'numerical' &&
!($this->is_valid_number($trimmedanswer) || $trimmedanswer == '*')) {
$errors[$prefix.'answer['.$key.']'] =
get_string('answermustbenumberorstar', 'qtype_numerical');
}
if ($subquestion->fraction[$key] == 1) {
$maxgrade = true;
}
if ($subquestion->fraction[$key] > $maxfraction) {
$maxfraction = $subquestion->fraction[$key];
}
// For 'multiresponse' we are OK if there is at least one fraction > 0.
if ($subquestion->qtype == 'multichoice' && $subquestion->single == 0 &&
$subquestion->fraction[$key] > 0) {
$maxgrade = true;
}
}
}
if ($subquestion->qtype == 'multichoice' && $answercount < 2) {
$errors[$prefix.'answer[0]'] = get_string('notenoughanswers', 'qtype_multichoice', 2);
} else if ($answercount == 0) {
$errors[$prefix.'answer[0]'] = get_string('notenoughanswers', 'question', 1);
}
if ($maxgrade == false) {
$errors[$prefix.'fraction[0]'] = get_string('fractionsnomax', 'question');
}
$sub++;
}
} else {
$errors['questiontext'] = get_string('questionsmissing', 'qtype_multianswer');
}
}
$errors = array_merge($errors, qtype_multianswer_validate_question($questiondisplay));
if (($this->negativediff > 0 || $this->usedinquiz &&
($this->negativediff > 0 || $this->negativediff < 0 ||

1
question/type/multianswer/lang/en/qtype_multianswer.php
View File

@ -28,6 +28,7 @@ $string['confirmsave'] = 'Confirm then save {$a}';
$string['correctanswer'] = 'Correct answer';
$string['correctanswerandfeedback'] = 'Correct answer and feedback';
$string['decodeverifyquestiontext'] = 'Decode and verify the question text';
$string['invalidmultianswerquestion'] = 'Invalid embedded answers (Cloze) question ({$a}).';
$string['layout'] = 'Layout';
$string['layouthorizontal'] = 'Horizontal row of radio-buttons';
$string['layoutmultiple_horizontal'] = 'Horizontal row of checkboxes';

63
question/type/multianswer/questiontype.php
View File

@ -28,6 +28,7 @@ defined('MOODLE_INTERNAL') || die();
require_once($CFG->dirroot . '/question/type/questiontypebase.php');
require_once($CFG->dirroot . '/question/type/multichoice/question.php');
require_once($CFG->dirroot . '/question/type/numerical/questiontype.php');
/**
* The multi-answer question type class.
@ -515,3 +516,65 @@ function qtype_multianswer_extract_question($text) {
}
return $question;
}
/**
* Validate a multianswer question.
*
* @param object $question The multianswer question to validate as returned by qtype_multianswer_extract_question
* @return array Array of error messages with questions field names as keys.
*/
function qtype_multianswer_validate_question(stdClass $question) : array {
$errors = array();
if (!isset($question->options->questions)) {
$errors['questiontext'] = get_string('questionsmissing', 'qtype_multianswer');
} else {
$subquestions = fullclone($question->options->questions);
if (count($subquestions)) {
$sub = 1;
foreach ($subquestions as $subquestion) {
$prefix = 'sub_'.$sub.'_';
$answercount = 0;
$maxgrade = false;
$maxfraction = -1;
foreach ($subquestion->answer as $key => $answer) {
if (is_array($answer)) {
$answer = $answer['text'];
}
$trimmedanswer = trim($answer);
if ($trimmedanswer !== '') {
$answercount++;
if ($subquestion->qtype == 'numerical' &&
!(qtype_numerical::is_valid_number($trimmedanswer) || $trimmedanswer == '*')) {
$errors[$prefix.'answer['.$key.']'] =
get_string('answermustbenumberorstar', 'qtype_numerical');
}
if ($subquestion->fraction[$key] == 1) {
$maxgrade = true;
}
if ($subquestion->fraction[$key] > $maxfraction) {
$maxfraction = $subquestion->fraction[$key];
}
// For 'multiresponse' we are OK if there is at least one fraction > 0.
if ($subquestion->qtype == 'multichoice' && $subquestion->single == 0 &&
$subquestion->fraction[$key] > 0) {
$maxgrade = true;
}
}
}
if ($subquestion->qtype == 'multichoice' && $answercount < 2) {
$errors[$prefix.'answer[0]'] = get_string('notenoughanswers', 'qtype_multichoice', 2);
} else if ($answercount == 0) {
$errors[$prefix.'answer[0]'] = get_string('notenoughanswers', 'question', 1);
}
if ($maxgrade == false) {
$errors[$prefix.'fraction[0]'] = get_string('fractionsnomax', 'question');
}
$sub++;
}
} else {
$errors['questiontext'] = get_string('questionsmissing', 'qtype_multianswer');
}
}
return $errors;
}

2
question/type/multianswer/version.php
View File

@ -26,7 +26,7 @@
defined('MOODLE_INTERNAL') || die();
$plugin->component = 'qtype_multianswer';
$plugin->version = 2018120300;
$plugin->version = 2019022500;
$plugin->requires = 2018112800;
$plugin->dependencies = array(

17
question/type/numerical/edit_numerical_form.php
View File

@ -319,22 +319,7 @@ class qtype_numerical_edit_form extends question_edit_form {
* @return bool whether this is a valid answer.
*/
protected function is_valid_answer($answer, $data) {
return $answer == '*' || $this->is_valid_number($answer);
}
/**
* Validate that a string is a nubmer formatted correctly for the current locale.
* @param string $x a string
* @return bool whether $x is a number that the numerical question type can interpret.
*/
protected function is_valid_number($x) {
if (is_null($this->ap)) {
$this->ap = new qtype_numerical_answer_processor(array());
}
list($value, $unit) = $this->ap->apply_units($x);
return !is_null($value) && !$unit;
return $answer == '*' || qtype_numerical::is_valid_number($answer);
}
/**

11
question/type/numerical/questiontype.php
View File

@ -52,6 +52,17 @@ class qtype_numerical extends question_type {
const UNITGRADEDOUTOFMARK = 1;
const UNITGRADEDOUTOFMAX = 2;
/**
* Validate that a string is a number formatted correctly for the current locale.
* @param string $x a string
* @return bool whether $x is a number that the numerical question type can interpret.
*/
public static function is_valid_number(string $x) : bool {
$ap = new qtype_numerical_answer_processor(array());
list($value, $unit) = $ap->apply_units($x);
return !is_null($value) && !$unit;
}
public function get_question_options($question) {
global $CFG, $DB, $OUTPUT;
parent::get_question_options($question);

81
question/type/numerical/tests/form_test.php
View File

@ -1,81 +0,0 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
* Unit tests for (some of) question/type/numerical/edit_numerical_form.php.
*
* @package qtype
* @subpackage numerical
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/question/type/numerical/edit_numerical_form.php');
/**
* Test sub-class, so we can force the locale.
*
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class test_qtype_numerical_edit_form extends qtype_numerical_edit_form {
public function __construct() {
// Warning, avoid running the parent constructor. That means the form is
// not properly tested but for now that is OK, we are only testing a few
// methods.
$this->ap = new qtype_numerical_answer_processor(array(), false, ',', ' ');
}
public function is_valid_number($x) {
return parent::is_valid_number($x);
}
}
/**
* Unit tests for question/type/numerical/edit_numerical_form.php.
*
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_numerical_form_test extends advanced_testcase {
public static $includecoverage = array(
'question/type/numerical/edit_numerical_form.php'
);
protected $form;
protected function setUp() {
$this->form = new test_qtype_numerical_edit_form();
}
protected function tearDown() {
$this->form = null;
}
public function test_is_valid_number() {
$this->assertTrue($this->form->is_valid_number('1,001'));
$this->assertTrue($this->form->is_valid_number('1.001'));
$this->assertTrue($this->form->is_valid_number('1'));
$this->assertTrue($this->form->is_valid_number('1,e8'));
$this->assertFalse($this->form->is_valid_number('1001 xxx'));
$this->assertTrue($this->form->is_valid_number('1.e8'));
}
}

9
question/type/numerical/tests/questiontype_test.php
View File

@ -169,4 +169,13 @@ class qtype_numerical_test extends advanced_testcase {
}
}
}
public function test_is_valid_number() {
$this->assertTrue(qtype_numerical::is_valid_number('1,001'));
$this->assertTrue(qtype_numerical::is_valid_number('1.001'));
$this->assertTrue(qtype_numerical::is_valid_number('1'));
$this->assertTrue(qtype_numerical::is_valid_number('1,e8'));
$this->assertFalse(qtype_numerical::is_valid_number('1001 xxx'));
$this->assertTrue(qtype_numerical::is_valid_number('1.e8'));
}
}

7
question/upgrade.txt
View File

@ -1,5 +1,12 @@
This files describes API changes for code that uses the question API.
=== 3.7 ===
The code for the is_valid_number function that was duplicated in the
qtype_numerical and qtype_multianswer plugins in the qtype_numerical_edit_form
and qtype_multianswer_edit_form classes has been moved to a static function
in the qtype_numerical class of the qtype_numerical plugin.
=== 3.5 ===
1) The question format exportprocess function now adds a