diff --git a/CHANGELOG.md b/CHANGELOG.md index 253ac7eaf06..4d55d088890 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Fixed `Phalcon\Http\Request::getHeaders` to handle auth headers correctly [#12480](https://github.com/phalcon/cphalcon/issues/12480) - Fixed `Phalcon\Http\Request::getMethod` to handle `X-HTTP-Method-Override` header correctly [#12478](https://github.com/phalcon/cphalcon/issues/12478) - Fixed `Phalcon\Mvc\Model\Criteria::limit` and `Phalcon\Mvc\Model\Query\Builder::limit` to work with `limit` and `offset` properly [#12419](https://github.com/phalcon/cphalcon/issues/12419) +- Fixed `Phalcon\Forms\Form` to correct form validation and set messages for elements [#12465](https://github.com/phalcon/cphalcon/issues/12465), [#11500](https://github.com/phalcon/cphalcon/issues/11500), [#11135](https://github.com/phalcon/cphalcon/issues/11135), [#3167](https://github.com/phalcon/cphalcon/issues/3167), [#12395](https://github.com/phalcon/cphalcon/issues/12395) # [3.0.2](https://github.com/phalcon/cphalcon/releases/tag/v3.0.2) (2016-11-26) - Fixed saving snapshot data while caching model [#12170](https://github.com/phalcon/cphalcon/issues/12170), [#12000](https://github.com/phalcon/cphalcon/issues/12000) diff --git a/phalcon/forms/form.zep b/phalcon/forms/form.zep index e8f2105fff0..e31d1b8d9ba 100644 --- a/phalcon/forms/form.zep +++ b/phalcon/forms/form.zep @@ -20,6 +20,7 @@ namespace Phalcon\Forms; use Phalcon\Validation; +use Phalcon\ValidationInterface; use Phalcon\DiInterface; use Phalcon\FilterInterface; use Phalcon\Di\Injectable; @@ -255,9 +256,9 @@ class Form extends Injectable implements \Countable, \Iterator */ public function isValid(var data = null, var entity = null) -> boolean { - var notFailed, messages, element, - validators, name, preparedValidators, filters, - validator, validation, elementMessages; + var validationStatus, messages, element, + validators, name, filters, + validator, validation, elementMessage; if empty this->_elements { return true; @@ -290,17 +291,19 @@ class Form extends Injectable implements \Countable, \Iterator } } - let notFailed = true, - messages = []; + let validationStatus = true; + + let validation = this->getValidation(); + + if typeof validation != "object" || !(validation instanceof ValidationInterface) { + // Create an implicit validation + let validation = new Validation(); + } for element in this->_elements { let validators = element->getValidators(); - if typeof validators != "array" { - continue; - } - - if count(validators) == 0 { + if typeof validators != "array" || count(validators) == 0 { continue; } @@ -309,25 +312,11 @@ class Form extends Injectable implements \Countable, \Iterator */ let name = element->getName(); - /** - * Prepare the validators - */ - let preparedValidators = []; - + /** + * Append (not overriding) element validators to validation class + */ for validator in validators { - let preparedValidators[] = [name, validator]; - } - - let validation = this->getValidation(); - - if typeof validation == "object" { - if validation instanceof Validation { - // Set the validators to be validated - validation->setValidators(preparedValidators); - } - } else { - // Create an implicit validation - let validation = new Validation(preparedValidators); + validation->add(name, validator); } /** @@ -341,21 +330,25 @@ class Form extends Injectable implements \Countable, \Iterator if typeof filters == "array" { validation->setFilters(name, filters); } - - /** - * Perform the validation - */ - let elementMessages = validation->validate(data, entity); - if count(elementMessages) { - let messages[name] = elementMessages; - let notFailed = false; - } } + /** + * Perform the validation + */ + let messages = validation->validate(data, entity); + if messages->count() { + // Add validation messages to relevant elements + for elementMessage in iterator(messages) { + this->get(elementMessage->getField())->appendMessage(elementMessage); + } + messages->rewind(); + let validationStatus = false; + } + /** * If the validation fails update the messages */ - if !notFailed { + if !validationStatus { let this->_messages = messages; } @@ -369,7 +362,7 @@ class Form extends Injectable implements \Countable, \Iterator /** * Return the validation status */ - return notFailed; + return validationStatus; } /** @@ -377,25 +370,14 @@ class Form extends Injectable implements \Countable, \Iterator */ public function getMessages(boolean byItemName = false) -> { - var messages, group, elementMessages; + var messages; let messages = this->_messages; - if byItemName { - if typeof messages != "array" { - return new Group(); - } - return messages; - } - - let group = new Group(); - - if typeof messages == "array" { - for elementMessages in messages { - group->appendMessages(elementMessages); - } + if typeof messages == "object" && messages instanceof Group { + return messages; } - return group; + return new Group(); } /** @@ -403,17 +385,10 @@ class Form extends Injectable implements \Countable, \Iterator */ public function getMessagesFor(string! name) -> { - var messages, elementMessages, group; - - let messages = this->_messages; - if fetch elementMessages, messages[name] { - return elementMessages; - } - - let group = new Group(), - this->_messages[name] = group; - - return group; + if this->has(name) { + return this->get(name)->getMessages(); + } + return new Group(); } /** @@ -712,7 +687,11 @@ class Form extends Injectable implements \Countable, \Iterator public function rewind() -> void { let this->_position = 0; - let this->_elementsIndexed = array_values(this->_elements); + if typeof this->_elements == "array" { + let this->_elementsIndexed = array_values(this->_elements); + } else { + let this->_elementsIndexed = []; + } } /** diff --git a/tests/unit/Forms/FormTest.php b/tests/unit/Forms/FormTest.php index 5b50108e98f..5965e321f5a 100755 --- a/tests/unit/Forms/FormTest.php +++ b/tests/unit/Forms/FormTest.php @@ -13,6 +13,7 @@ use Phalcon\Validation\Validator\PresenceOf; use Phalcon\Validation\Validator\Regex; use Phalcon\Validation\Validator\StringLength; +use Phalcon\Validation; /** * \Phalcon\Test\Unit\Forms\FormTest @@ -398,4 +399,145 @@ function () { } ); } + + /** + * Tests Element::hasMessages() Element::getMessages() + * + * @author Mohamad Rostami + * @issue 11135, 3167 + */ + public function testElementMessages() + { + $this->specify('Element messages are empty if form validation fails', function () { + // First element + $telephone = new Text('telephone'); + + $telephone->addValidators([ + new Regex([ + 'pattern' => '/\+44 [0-9]+ [0-9]+/', + 'message' => 'The telephone has an invalid format' + ]) + ]); + + // Second element + $address = new Text('address'); + $form = new Form(); + + $form->add($telephone); + $form->add($address); + + expect($form->isValid(['telephone' => '12345', 'address' => 'hello']))->false(); + expect($form->get('telephone')->hasMessages())->true(); + expect($form->get('address')->hasMessages())->false(); + + expect($form->get('telephone')->getMessages())->equals( + Group::__set_state([ + '_messages' => [ + Message::__set_state([ + '_type' => 'Regex', + '_message' => 'The telephone has an invalid format', + '_field' => 'telephone', + '_code' => 0, + ]) + ], + ]) + ); + expect($form->get('telephone')->getMessages())->equals($form->getMessages()); + expect($form->get('address')->getMessages())->equals(Group::__set_state(['_messages' => []])); + expect($form->getMessagesFor('notelement'))->equals(Group::__set_state(['_messages' => []])); + }); + } + + /** + * Tests Form::setValidation() + * + * @author Mohamad Rostami + * @issue 12465 + */ + public function testCustomValidation() + { + $this->specify('Injecting custom validation to form doesn\'t validate correctly', function () { + // First element + $telephone = new Text('telephone'); + $customValidation = new Validation(); + $customValidation->add('telephone', new Regex([ + 'pattern' => '/\+44 [0-9]+ [0-9]+/', + 'message' => 'The telephone has an invalid format' + ])); + $form = new Form(); + $address = new Text('address'); + $form->add($telephone); + $form->add($address); + $form->setValidation($customValidation); + expect($form->isValid(['telephone' => '12345', 'address' => 'hello']))->false(); + expect($form->get('telephone')->hasMessages())->true(); + expect($form->get('address')->hasMessages())->false(); + expect($form->get('telephone')->getMessages())->equals( + Group::__set_state([ + '_messages' => [ + Message::__set_state([ + '_type' => 'Regex', + '_message' => 'The telephone has an invalid format', + '_field' => 'telephone', + '_code' => 0, + ]) + ], + ]) + ); + expect($form->get('telephone')->getMessages())->equals($form->getMessages()); + expect($form->get('address')->getMessages())->equals(Group::__set_state(['_messages' => []])); + }); + } + + /** + * Tests Form::isValid() + * + * @author Mohamad Rostami + * @issue 11500 + */ + public function testMergeValidators() + { + $this->specify('Injecting custom validation to form doesn\'t merge validators on isValid', function () { + // First element + $telephone = new Text('telephone'); + $telephone->addValidators([ + new PresenceOf([ + 'message' => 'The telephone is required' + ]) + ]); + $customValidation = new Validation(); + $customValidation->add('telephone', new Regex([ + 'pattern' => '/\+44 [0-9]+ [0-9]+/', + 'message' => 'The telephone has an invalid format' + ])); + $form = new Form(); + $address = new Text('address'); + $form->add($telephone); + $form->add($address); + $form->setValidation($customValidation); + expect($form->isValid(['address' => 'hello']))->false(); + expect($form->get('telephone')->hasMessages())->true(); + expect($form->get('address')->hasMessages())->false(); + expect($form->get('telephone')->getMessages())->equals( + Group::__set_state([ + '_messages' => [ + Message::__set_state([ + '_type' => 'Regex', + '_message' => 'The telephone has an invalid format', + '_field' => 'telephone', + '_code' => 0, + ]), + Message::__set_state([ + '_type' => 'PresenceOf', + '_message' => 'The telephone is required', + '_field' => 'telephone', + '_code' => 0, + ]) + ], + ]) + ); + expect($form->get('telephone')->getMessages())->equals($form->getMessages()); + expect($form->get('address')->getMessages())->equals(Group::__set_state(['_messages' => []])); + }); + } }