From 9bac67782656e2bba84df4f8658ef9fca4fc92cc Mon Sep 17 00:00:00 2001
From: Daniel Thee Roperto <danielroperto@catalyst-au.net>
Date: Tue, 4 Oct 2016 19:05:13 +1100
Subject: [PATCH] MDL-56250 forms library: Changed form to validate once per
 object.

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.
---
 lib/formslib.php            | 10 ++++---
 lib/tests/formslib_test.php | 53 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/lib/formslib.php b/lib/formslib.php
index 19b15bb8958..0ee1e82d08f 100644
--- a/lib/formslib.php
+++ b/lib/formslib.php
@@ -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.
@@ -539,11 +542,10 @@ abstract class moodleform {
      * @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();
@@ -581,9 +583,9 @@ abstract class moodleform {
                 $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;
     }
 
     /**
diff --git a/lib/tests/formslib_test.php b/lib/tests/formslib_test.php
index fb44b9138cf..f7c5ed90cd5 100644
--- a/lib/tests/formslib_test.php
+++ b/lib/tests/formslib_test.php
@@ -606,6 +606,25 @@ class core_formslib_testcase extends advanced_testcase {
         $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());
+    }
 }
 
 
@@ -928,4 +947,36 @@ class formslib_persistantrreeze_element extends moodleform {
         $mform->addElement('text', 'textnotpersistant', 'test', 'test');
         $mform->setType('textnotpersistant', PARAM_TEXT);
     }
-}
\ No newline at end of file
+}
+
+/**
+ * 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;
+    }
+}