Skip to content

Commit

Permalink
MDL-56250 forms library: Changed form to validate once per object.
Browse files Browse the repository at this point in the history
If two instances of moodleforms are validated in the same run, most
likely a phpunit test run, it would store the first validation result.
Now it stores the validation result per instance, not for all instances.
  • Loading branch information
Daniel Thee Roperto committed Oct 20, 2016
1 parent 2afadc0 commit 9bac677
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 5 deletions.
10 changes: 6 additions & 4 deletions lib/formslib.php
Expand Up @@ -139,6 +139,9 @@ abstract class moodleform {
/** @var object definition_after_data executed flag */
protected $_definition_finalized = false;

/** @var bool|null stores the validation result of this form or null if not yet validated */
protected $_validated = null;

/**
* The constructor function calls the abstract function definition() and it will then
* process and clean and attempt to validate incoming data.
Expand Down Expand Up @@ -539,11 +542,10 @@ function is_validated() {
* @return bool true if form data valid
*/
function validate_defined_fields($validateonnosubmit=false) {
static $validated = null; // one validation is enough
$mform =& $this->_form;
if ($this->no_submit_button_pressed() && empty($validateonnosubmit)){
return false;
} elseif ($validated === null) {
} elseif ($this->_validated === null) {
$internal_val = $mform->validate();

$files = array();
Expand Down Expand Up @@ -581,9 +583,9 @@ function validate_defined_fields($validateonnosubmit=false) {
$moodle_val = true;
}

$validated = ($internal_val and $moodle_val and $file_val);
$this->_validated = ($internal_val and $moodle_val and $file_val);
}
return $validated;
return $this->_validated;
}

/**
Expand Down
53 changes: 52 additions & 1 deletion lib/tests/formslib_test.php
Expand Up @@ -606,6 +606,25 @@ public function test_persistantrreeze_element() {
$this->assertNotTag(array('id' => 'id_textfrozen_persistant'), $html);

}

/**
* Ensure a validation can run at least once per object. See MDL-56259.
*/
public function test_multiple_validation() {
$this->resetAfterTest(true);

// It should be valid.
formslib_multiple_validation_form::mock_submit(['somenumber' => '10']);
$form = new formslib_multiple_validation_form();
$this->assertTrue($form->is_validated());
$this->assertEquals(10, $form->get_data()->somenumber);

// It should not validate.
formslib_multiple_validation_form::mock_submit(['somenumber' => '-5']);
$form = new formslib_multiple_validation_form();
$this->assertFalse($form->is_validated());
$this->assertNull($form->get_data());
}
}


Expand Down Expand Up @@ -928,4 +947,36 @@ public function definition() {
$mform->addElement('text', 'textnotpersistant', 'test', 'test');
$mform->setType('textnotpersistant', PARAM_TEXT);
}
}
}

/**
* Used to test that you can validate a form more than once. See MDL-56250.
* @package core_form
* @author Daniel Thee Roperto <daniel.roperto@catalyst-au.net>
* @copyright 2016 Catalyst IT
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class formslib_multiple_validation_form extends moodleform {
/**
* Simple definition, one text field which can have a number.
*/
public function definition() {
$mform = $this->_form;
$mform->addElement('text', 'somenumber');
$mform->setType('somenumber', PARAM_INT);
}

/**
* The number cannot be negative.
* @param array $data An array of form data
* @param array $files An array of form files
* @return array Error messages
*/
public function validation($data, $files) {
$errors = parent::validation($data, $files);
if ($data['somenumber'] < 0) {
$errors['somenumber'] = 'The number cannot be negative.';
}
return $errors;
}
}

0 comments on commit 9bac677

Please sign in to comment.