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

MDL-46148 qtype_calculated: fix validation when importing.

Browse Source 
In order to do this in a sane way, I cleaned up a lot of old mess,
inclduing:

1. Previously, qtype_calcuated used ->answeres when importing, and
->answer when saving the form. This was crazy, so I fixed it, and
stripped out the code that made the alternative variable name work.

2. Similarly, it could handle ->answer being either an array, such as
you would get form the HTML editor, or a simple string, which is what
you get form the form. I simplified that too.

3. Finally, I made import use a transaction around saving each
question, so we don't get half questions in the database when an error
occurs.
This commit is contained in:
Tim Hunt
2014-07-09 14:11:42 +01:00
committed by Dan Poltawski
parent f8a6c3d6f0
commit 7bf7ef3919
6 changed files with 51 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
question/format.php
+6
View File
@@ -357,6 +357,7 @@ class qformat_default {
$count = 0;
foreach ($questions as $question) { // Process and store each question
$transaction = $DB->start_delegated_transaction();
// reset the php timeout
set_time_limit(0);
@@ -429,9 +430,14 @@ class qformat_default {
if (!empty($result->error)) {
echo $OUTPUT->notification($result->error);
// Can't use $transaction->rollback(); since it requires an exception,
// and I don't want to rewrite this code to change the error handling now.
$DB->force_transaction_rollback();
return false;
}
$transaction->allow_commit();
if (!empty($result->notice)) {
echo $OUTPUT->notification($result->notice);
return true;
question/format/webct/format.php
+2 -5
View File
@@ -606,12 +606,9 @@ class qformat_webct extends qformat_default {
// Calculated Question.
$question = $this->defaultquestion();
$question->qtype = 'calculated';
$question->answers = array(); // No problem as they go as :FORMULA: from webct.
$question->answer = array(); // No problem as they go as :FORMULA: from webct.
$question->units = array();
$question->dataset = array();
// To make us pass the end-of-question sanity checks.
$question->answer = array('dummy');
$question->fraction = array('1.0');
$question->feedback = array();
@@ -737,7 +734,7 @@ class qformat_webct extends qformat_default {
if (preg_match('~^:FORMULA:(.*)~i', $line, $webctoptions)) {
// Answer for a calculated question.
++$currentchoice;
$question->answers[$currentchoice] =
$question->answer[$currentchoice] =
qformat_webct_convert_formula($webctoptions[1]);
// Default settings.
question/format/xml/format.php
+2 -2
View File
@@ -775,7 +775,7 @@ class qformat_xml extends qformat_default {
// Get answers array.
$answers = $question['#']['answer'];
$qo->answers = array();
$qo->answer = array();
$qo->feedback = array();
$qo->fraction = array();
$qo->tolerance = array();
@@ -789,7 +789,7 @@ class qformat_xml extends qformat_default {
if (empty($ans->answer['text'])) {
$ans->answer['text'] = '*';
}
$qo->answers[] = $ans->answer;
$qo->answer[] = $ans->answer['text'];
$qo->feedback[] = $ans->feedback;
$qo->tolerance[] = $answer['#']['tolerance'][0]['#'];
// Fraction as a tag is deprecated.
question/type/calculated/questiontype.php
+27 -27
View File
@@ -131,11 +131,13 @@ class qtype_calculated extends question_type {
public function save_question_options($question) {
global $CFG, $DB;
// Make it impossible to save bad formulas anywhere.
$this->validate_question_data($question);
// The code is used for calculated, calculatedsimple and calculatedmulti qtypes.
$context = $question->context;
if (isset($question->answer) && !isset($question->answers)) {
$question->answers = $question->answer;
}
// Calculated options.
$update = true;
$options = $DB->get_record('question_calculated_options',
@@ -185,14 +187,7 @@ class qtype_calculated extends question_type {
$units = $result->units;
}
// Insert all the new answers.
if (isset($question->answer) && !isset($question->answers)) {
$question->answers = $question->answer;
}
foreach ($question->answers as $key => $answerdata) {
if (is_array($answerdata)) {
$answerdata = $answerdata['text'];
}
foreach ($question->answer as $key => $answerdata) {
if (trim($answerdata) == '') {
continue;
}
@@ -471,6 +466,25 @@ class qtype_calculated extends question_type {
}
}
/**
* Validate data before save.
* @param stdClass $question data from the form / import file.
*/
protected function validate_question_data($question) {
$this->validate_text($question->questiontext); // Yes, really no ['text'].
if (isset($question->generalfeedback['text'])) {
$this->validate_text($question->generalfeedback['text']);
} else if (isset($question->generalfeedback)) {
$this->validate_text($question->generalfeedback); // Because question import is weird.
}
foreach ($question->answer as $key => $answer) {
$this->validate_answer($answer);
$this->validate_text($question->feedback[$key]['text']);
}
}
/**
* This method prepare the $datasets in a format similar to dadatesetdefinitions_form.php
* so that they can be saved
@@ -483,13 +497,13 @@ class qtype_calculated extends question_type {
* @param object $form
* @param int $questionfromid default = '0'
*/
public function preparedatasets($form , $questionfromid = '0') {
public function preparedatasets($form, $questionfromid = '0') {
// The dataset names present in the edit_question_form and edit_calculated_form
// are retrieved.
$possibledatasets = $this->find_dataset_names($form->questiontext);
$mandatorydatasets = array();
foreach ($form->answers as $key => $answer) {
foreach ($form->answer as $key => $answer) {
$mandatorydatasets += $this->find_dataset_names($answer);
}
// If there are identical datasetdefs already saved in the original question
@@ -579,12 +593,6 @@ class qtype_calculated extends question_type {
public function save_question($question, $form) {
global $DB;
if (isset($form->correctfeedback)) {
$this->validate_text($form->correctfeedback['text']);
$this->validate_text($form->partiallycorrectfeedback['text']);
$this->validate_text($form->incorrectfeedback['text']);
}
if ($this->wizardpagesnumber() == 1 || $question->qtype == 'calculatedsimple') {
$question = parent::save_question($question, $form);
return $question;
@@ -605,14 +613,6 @@ class qtype_calculated extends question_type {
case '' :
case 'question': // Coming from the first page, creating the second.
if (empty($form->id)) { // or a new question $form->id is empty.
// Make it impossible to save bad formulas anywhere.
$this->validate_text($form->questiontext['text']);
$this->validate_text($form->generalfeedback['text']);
foreach ($form->answer as $key => $answer) {
$this->validate_answer($answer);
$this->validate_text($form->feedback[$key]['text']);
}
$question = parent::save_question($question, $form);
// Prepare the datasets using default $questionfromid.
$this->preparedatasets($form);
question/type/calculatedmulti/questiontype.php
+11 -4
View File
@@ -42,6 +42,9 @@ class qtype_calculatedmulti extends qtype_calculated {
global $CFG, $DB;
$context = $question->context;
// Make it impossible to save bad formulas anywhere.
$this->validate_question_data($question);
// Calculated options.
$update = true;
$options = $DB->get_record('question_calculated_options',
@@ -72,10 +75,7 @@ class qtype_calculatedmulti extends qtype_calculated {
}
// Insert all the new answers.
if (isset($question->answer) && !isset($question->answers)) {
$question->answers = $question->answer;
}
foreach ($question->answers as $key => $answerdata) {
foreach ($question->answer as $key => $answerdata) {
if (is_array($answerdata)) {
$answerdata = $answerdata['text'];
}
@@ -163,6 +163,13 @@ class qtype_calculatedmulti extends qtype_calculated {
}
}
protected function validate_question_data($question) {
parent::validate_question_data($question);
$this->validate_text($question->correctfeedback['text']);
$this->validate_text($question->partiallycorrectfeedback['text']);
$this->validate_text($question->incorrectfeedback['text']);
}
protected function make_question_instance($questiondata) {
question_bank::load_question_definition_classes($this->name());
if ($questiondata->options->single) {
question/type/calculatedsimple/questiontype.php
+3 -8
View File
@@ -43,11 +43,9 @@ class qtype_calculatedsimple extends qtype_calculated {
public function save_question_options($question) {
global $CFG, $DB;
$context = $question->context;
// Get old answers.
if (isset($question->answer) && !isset($question->answers)) {
$question->answers = $question->answer;
}
// Make it impossible to save bad formulas anywhere.
$this->validate_question_data($question);
// Get old versions of the objects.
if (!$oldanswers = $DB->get_records('question_answers',
@@ -69,10 +67,7 @@ class qtype_calculatedsimple extends qtype_calculated {
$units = &$result->units;
}
// Insert all the new answers.
if (isset($question->answer) && !isset($question->answers)) {
$question->answers = $question->answer;
}
foreach ($question->answers as $key => $answerdata) {
foreach ($question->answer as $key => $answerdata) {
if (is_array($answerdata)) {
$answerdata = $answerdata['text'];
}