L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-63905 qtype_multianswer: validate imported questions

Browse Source
This commit is contained in:
Jean-Michel Vedrine
2019-03-06 08:36:43 +01:00
parent b38eb7882b
commit ffe4e082b0
14 changed files with 190 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
question/format/multianswer/format.php
+7
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;
question/format/multianswer/tests/fixtures/broken_multianswer_1.txt
Vendored
+1
View File
@@ -0,0 +1 @@
Please select the fruits {1:MULTICHOICE:=Apple#Correct}
question/format/multianswer/tests/fixtures/broken_multianswer_2.txt
Vendored
+1
View File
@@ -0,0 +1 @@
Please select the fruits {1:MULTICHOICE:Pear#Incorrect~%50%Apple#Correct}
question/format/multianswer/tests/fixtures/broken_multianswer_3.txt
Vendored
+1
View File
@@ -0,0 +1 @@
What grade would you give it? {3:NUMERICAL:=zero}
question/format/multianswer/tests/fixtures/broken_multianswer_4.txt
Vendored
+1
View File
@@ -0,0 +1 @@
Please select the fruits.
question/format/multianswer/tests/multianswerformat_test.php
+77
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);
}
}
question/format/xml/format.php
+14 -5
View File
@@ -237,7 +237,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'])) {
@@ -472,6 +472,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';
@@ -480,8 +485,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'])) {
@@ -511,7 +520,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'])) {
@@ -926,7 +935,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);
question/type/multianswer/edit_multianswer_form.php
+2 -50
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'));
@@ -477,55 +477,7 @@ class qtype_multianswer_edit_form extends question_edit_form {
$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 ||
question/type/multianswer/lang/en/qtype_multianswer.php
+1
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';
question/type/multianswer/questiontype.php
+63
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.
@@ -509,3 +510,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($question) {
$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;
}
question/type/multianswer/version.php
+1 -1
View File
@@ -26,7 +26,7 @@
defined('MOODLE_INTERNAL') || die();
$plugin->component = 'qtype_multianswer';
$plugin->version = 2018051400;
$plugin->version = 2018051401;
$plugin->requires = 2018050800;
$plugin->dependencies = array(
question/type/numerical/edit_numerical_form.php
+1 -1
View File
@@ -319,7 +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);
return $answer == '*' || qtype_numerical::is_valid_number($answer);
}
/**
question/type/numerical/questiontype.php
+11
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($x) {
$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);
question/type/numerical/tests/questiontype_test.php
+9
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'));
}
}